Re: [libvirt] Question about html file in docs
On Wed, Aug 13, 2008 at 09:56:00AM +0900, Atsushi SAKAI wrote: Hi, I think html files in docs directory are redundunt. It is because html file is created by html.in. May I remove these files? No Or are there any reason about staying these files? Because the web site is a CVS checkout of the docs/ subdir Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virt-install?
Hi, I got the libvirt source code (cvs), and compiled. However, I cannot find the virt-install anywhere. Is it included inside libvirt? Thank you, Jun -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: virDomainMemoryPeek for Xen?
On Wed, Aug 13, 2008 at 01:19:42PM +0900, Jun Koi wrote: According to the comment in your driver code for virDomainMemoryPeek, libvirt is not currently supporting Xen. Why is that? As far as I see, Xen is use the same GPL2 version as libvirt. I am thinking about implementing the driver for Xen, so have this question. It'd be really good to have a driver for this for libvirt. QEMU does virtual-physical translation for us, through the guest's CR3 page tables. [This discussion will concentrate on x86 for the moment :-)] If you look at the qemu source, file target-i386/ helper2.c, function cpu_get_phys_page_debug, you will see the code they use to navigate through the 3 or 4 levels of page tables for i386 x86-64 respectively. The QEMU memsave command made it almost trivial to write virDomainMemoryPeek (...VIR_MEMORY_VIRTUAL...); On Xen things are a bit different. You can map in physical pages from another guest using the libxc call xc_map_foreign_range. Note that you cannot just call xc_map_foreign_range because the libxc libvirt licenses are _not_ compatible. So instead you'd need to do the underlying sequence of mmap / ioctl / munmap. (See tools/libxc/ xc_linux.c in the Xen source). But you still need to do virtual to physical page translation, either using the qemu source as an example, or using xc_translate_foreign_address as a guide (or just using the Intel Programmers Reference Manual and doing it from first principles). Another alternative is to implement virDomainMemoryPeek (...VIR_MEMORY_PHYSICAL...). It is not possible to implement this for QEMU at all, at least not without changing QEMU. Implementing this for Xen would be much easier because you don't need to do address translation, but it does push the problem of address translation up to the callers. Places to look in the QEMU source for inspiration: monitor.c:do_memory_save target-i386/helper2.c:cpu_get_phys_page_debug Other places to look in the Xen source for inspiration: tools/libxc/xc_linux.c tools/libxc/xc_pagetab.c tools/xentrace/xenctx.c (thanks Mark McLoughlin) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about html file in docs
Hi, Daniel If I run the make on libvirt, cvs diff outputs large diffs on html. This is because the difference of html and html.in is large. I think it should be fixed. How do you think? Thanks Atsushi SAKAI Daniel Veillard [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 09:56:00AM +0900, Atsushi SAKAI wrote: Hi, I think html files in docs directory are redundunt. It is because html file is created by html.in. May I remove these files? No Or are there any reason about staying these files? Because the web site is a CVS checkout of the docs/ subdir Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virt-install?
On Wed, Aug 13, 2008 at 04:35:00PM +0900, Jun Koi wrote: Hi, I got the libvirt source code (cvs), and compiled. However, I cannot find the virt-install anywhere. Is it included inside libvirt? No, a companion of virt-manager project see http://virt-manager.org/scmrepo.html to fetch the sources Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virt-install?
On Wed, Aug 13, 2008 at 4:43 PM, Daniel Veillard [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 04:35:00PM +0900, Jun Koi wrote: Hi, I got the libvirt source code (cvs), and compiled. However, I cannot find the virt-install anywhere. Is it included inside libvirt? No, a companion of virt-manager project see http://virt-manager.org/scmrepo.html to fetch the sources I got virtinstall, but there is no INSTALL file inside (should have one, or README should mention that). So how to install it?? Thanks, Jun -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about html file in docs
On Wed, Aug 13, 2008 at 03:34:33AM -0400, Daniel Veillard wrote: On Wed, Aug 13, 2008 at 09:56:00AM +0900, Atsushi SAKAI wrote: Hi, I think html files in docs directory are redundunt. It is because html file is created by html.in. May I remove these files? No Or are there any reason about staying these files? Because the web site is a CVS checkout of the docs/ subdir Could the cron script which updates the CVS checkout simply run 'make' in the docs/ directory ? It already has to run a couple of scripts to re-generate the NEWS ChangeLog.html files on every checkout 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] Re: virDomainMemoryPeek for Xen?
On Wed, Aug 13, 2008 at 08:33:24AM +0100, Richard W.M. Jones wrote: On Wed, Aug 13, 2008 at 01:19:42PM +0900, Jun Koi wrote: According to the comment in your driver code for virDomainMemoryPeek, libvirt is not currently supporting Xen. Why is that? As far as I see, Xen is use the same GPL2 version as libvirt. I am thinking about implementing the driver for Xen, so have this question. It'd be really good to have a driver for this for libvirt. Yes, it would be very useful indeed On Xen things are a bit different. You can map in physical pages from another guest using the libxc call xc_map_foreign_range. Note that you cannot just call xc_map_foreign_range because the libxc libvirt licenses are _not_ compatible. So instead you'd need to do the underlying sequence of mmap / ioctl / munmap. (See tools/libxc/ xc_linux.c in the Xen source). But you still need to do virtual to physical page translation, either using the qemu source as an example, or using xc_translate_foreign_address as a guide (or just using the Intel Programmers Reference Manual and doing it from first principles). Alot of work, but ultimately it has to be done if we want tools like virt-mem to be usable with Xen. Another alternative is to implement virDomainMemoryPeek (...VIR_MEMORY_PHYSICAL...). It is not possible to implement this for QEMU at all, at least not without changing QEMU. Implementing this for Xen would be much easier because you don't need to do address translation, but it does push the problem of address translation up to the callers. And it wouldn't help getting any of your guest debugging programs working bevacuse they all need VIR_MEMORY_VIRTUAL Places to look in the QEMU source for inspiration: monitor.c:do_memory_save target-i386/helper2.c:cpu_get_phys_page_debug Other places to look in the Xen source for inspiration: tools/libxc/xc_linux.c tools/libxc/xc_pagetab.c tools/xentrace/xenctx.c (thanks Mark McLoughlin) NB, just a another reminder - do not copy code from these files if doing a libvirt implementation for Xen since they are not license compatible. 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] virt-install?
On Wed, Aug 13, 2008 at 04:59:35PM +0900, Jun Koi wrote: On Wed, Aug 13, 2008 at 4:43 PM, Daniel Veillard [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 04:35:00PM +0900, Jun Koi wrote: Hi, I got the libvirt source code (cvs), and compiled. However, I cannot find the virt-install anywhere. Is it included inside libvirt? No, a companion of virt-manager project see http://virt-manager.org/scmrepo.html to fetch the sources I got virtinstall, but there is no INSTALL file inside (should have one, or README should mention that). So how to install it?? It uses the standard python installation procedure As root python setup.py install Or as non-root python setup.py install --prefix=$HOME/usr export PYTHONPATH=$HOME/usr/lib/python2.5/site-packages 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
[libvirt] [PATCH] get memory in openvz driver
OpenVZ has several parameters for memory management. All of them can be configured independetly. Patch allow to get configuration of memory from container config and then calculate total usage of memory. It is open question how to manage memory? Index: src/openvz_conf.c === RCS file: /data/cvs/libvirt/src/openvz_conf.c,v retrieving revision 1.33 diff -u -p -r1.33 openvz_conf.c --- src/openvz_conf.c 5 Aug 2008 10:53:05 - 1.33 +++ src/openvz_conf.c 13 Aug 2008 08:32:57 - @@ -138,6 +138,35 @@ strtoI(const char *str) return val; } +/* openvz memory parameters have pattern +456633:2345565 - barrier limit +function split to 2 separate value +*/ +int +splitValues(const char *value, unsigned long *barrier, unsigned long *limit) +{ +if (value == NULL) +return -1; + +if (sscanf(value, %lu:%lu , barrier, limit) != 2) +return -1; + +return 0; +} + +/* function return memory page size +*/ +static long +get_pagesize(void) +{ +long pagesize; +if ((pagesize = sysconf(_SC_PAGESIZE)) == -1) { +/*Unable to get page size*/ + pagesize = 4096; //set default +} +return pagesize; +} + /* function checks MAC address is empty return 0 - empty 1 - not @@ -814,4 +843,310 @@ int openvzAssignUUIDs(void) return 0; } +const char *ubc_names[] = { +KMEMSIZE, +LOCKEDPAGES, +PRIVVMPAGES, +SHMPAGES, +NUMPROC, +PHYSPAGES, +VMGUARPAGES, +OOMGUARPAGES, +NUMTCPSOCK, +NUMFLOCK, +NUMPTY, +NUMSIGINFO, +TCPSNDBUF, +TCPRCVBUF, +OTHERSOCKBUF, +DGRAMRCVBUF, +NUMOTHERSOCK, +NUMFILE, +DCACHESIZE, +NUMIPTENT, +AVNUMPROC, +NULL +}; + +/* function return list of parameters based on +* parameter names +* return pointer to openvz_param or NULL +*/ +static struct openvz_param* +openvzGetEmptyParams(const char *list_names[]) { +int i = 0, count; +struct openvz_param *params; + +/*get count*/ +while(list_names[i] != NULL) +i++; + +if (i == 0) +return NULL; + +count = i + 1; + +if (VIR_ALLOC_N(params, count) 0) /*could not allocate memory*/ +return NULL; + +i = 0; +while(list_names[i] != NULL) { +params[i].name = strdup(list_names[i]); +if (params[i].name == NULL) /*could not allocate memory*/ +break; + +params[i].value = NULL; +i++; +} + +params[i].name = NULL; +params[i].value = NULL; + +return params; +} + +static void +openvzFreeParams(struct openvz_param * params) { +int i = 0; + +if (params == NULL) +return; + +while(params[i].name != NULL) { +VIR_FREE(params[i].name); + +if (params[i].value != NULL) { + VIR_FREE(params[i].value); +} +i++; +} + +VIR_FREE(params); +} + +void +openvzFreeUbParam(struct openvz_ub_param *ub) +{ +if (ub == NULL) +return; +#define FREE_P(x) if (ub-x != NULL) VIR_FREE(ub-x); + +FREE_P(kmemsize) +FREE_P(lockedpages) +FREE_P(privvmpages) +FREE_P(shmpages) +FREE_P(numproc) +FREE_P(physpages) +FREE_P(vmguarpages) +FREE_P(oomguarpages) +FREE_P(numtcpsock) +FREE_P(numflock) +FREE_P(numpty) +FREE_P(numsiginfo) +FREE_P(tcpsndbuf) +FREE_P(tcprcvbuf) +FREE_P(othersockbuf) +FREE_P(dgramrcvbuf) +FREE_P(numothersock) +FREE_P(numfile) +FREE_P(dcachesize) +FREE_P(numiptent) +FREE_P(avnumproc) + +#undef FREE_P +VIR_FREE(ub); +} + +/* +* Read several parameters from container config +* return: -1 - error +* 0 - OK +* TODO: rewrite function to don't read config several times +*/ +static int +openvzReadMultiConfigParams(int vpsid, struct openvz_param *params) +{ +int i = 0; +char value[1024]; +int ret = 0; + +while(params[i].name != NULL) { + ret = openvzReadConfigParam(vpsid, params[i].name, value, 1024); + if (ret 0) + return -1; + if (ret 0) + params[i].value = strdup(value); + /*do nothing if parameter don't found*/ + + i++; +} + +return 0; +} + +/* search parameter in list and return it value +* return pointer to value or NULL +*/ +static char * +openvzGetParamValue(struct openvz_param *params, const char *name) +{ +int j; +for (j = 0; params[j].name != NULL; j++) { +if (STREQ(params[j].name, name)) { +return params[j].value; +} +} +return NULL; +} + +static int +openvzConvertParams2UbParams(struct openvz_param *params, + struct openvz_ub_param *ub) +{ +char *value; +if (ub == NULL || params == NULL) +return -1; + +#define PARAM2UB(param_name, ub_param_name) \ +if ((value = openvzGetParamValue(params, param_name)) != NULL) { \ + if (VIR_ALLOC(ub-ub_param_name) 0) \ + goto error; \ +
[libvirt] PATCH: 0/5: Enhancing the virExec function
The following series of patches do a bunch of work on the virExec function. Overall this leads to a more robust implementation with better error reporting, and an increased level functionality. This lets us remove all uses of fork/exec in libvirt and replace them with virExec. This was all motivated by some fork/exec bugs discovered when refactoring LXC driver which have potential to affect every use of fork/exec in our code. 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/5: Improved error reporting
There are several system calls in the virExec function for which we don't or can't report errors. This patch does a couple of things to improve the situation. It moves the code for setting non-block/close-exec to before the fork() so we can report errors for it. It removes the 'dom' and 'net' params from the ReportError function since we deprecated those long ago and all callers simply pass in NULL. It resets the 'virErrorHandler' callback to NULL in the child, so that errors raised will get reported to stderr instead of invoking a callback which is likely no longer valid in the child process. It reports failures to exec the binary and dup file descriptors. All errors in the child will end up on stderr, so they will at least be visible on the parent's console, or a logfile if one was setup for the child. Previously there would just be silent failure. Daniel diff -r 4f44b07c47c1 src/util.c --- a/src/util.cMon Aug 11 20:45:28 2008 +0100 +++ b/src/util.cTue Aug 12 14:52:41 2008 +0100 @@ -61,9 +61,7 @@ #ifndef PROXY static void ReportError(virConnectPtr conn, - virDomainPtr dom, - virNetworkPtr net, - int code, const char *fmt, ...) { +int code, const char *fmt, ...) { va_list args; char errorMessage[MAX_ERROR_LEN]; @@ -74,7 +72,7 @@ } else { errorMessage[0] = '\0'; } -__virRaiseError(conn, dom, net, VIR_FROM_NONE, code, VIR_ERR_ERROR, +__virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, code, VIR_ERR_ERROR, NULL, NULL, NULL, -1, -1, %s, errorMessage); } @@ -109,7 +107,7 @@ int pipeerr[2] = {-1,-1}; if ((null = open(_PATH_DEVNULL, O_RDONLY)) 0) { -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, +ReportError(conn, VIR_ERR_INTERNAL_ERROR, _(cannot open %s: %s), _PATH_DEVNULL, strerror(errno)); goto cleanup; @@ -117,13 +115,41 @@ if ((outfd != NULL pipe(pipeout) 0) || (errfd != NULL pipe(pipeerr) 0)) { -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, +ReportError(conn, VIR_ERR_INTERNAL_ERROR, _(cannot create pipe: %s), strerror(errno)); goto cleanup; } +if (outfd) { +if(non_block + virSetNonBlock(pipeout[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set non-blocking file descriptor flag)); +goto cleanup; +} + +if(virSetCloseExec(pipeout[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set close-on-exec file descriptor flag)); +goto cleanup; +} +} +if (errfd) { +if(non_block + virSetNonBlock(pipeerr[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set non-blocking file descriptor flag)); +goto cleanup; +} +if(virSetCloseExec(pipeerr[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set close-on-exec file descriptor flag)); +goto cleanup; +} +} + if ((pid = fork()) 0) { -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, +ReportError(conn, VIR_ERR_INTERNAL_ERROR, _(cannot fork child process: %s), strerror(errno)); goto cleanup; } @@ -132,26 +158,10 @@ close(null); if (outfd) { close(pipeout[1]); -if(non_block) -if(virSetNonBlock(pipeout[0]) == -1) -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, -_(Failed to set non-blocking file descriptor flag)); - -if(virSetCloseExec(pipeout[0]) == -1) -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, -_(Failed to set close-on-exec file descriptor flag)); *outfd = pipeout[0]; } if (errfd) { close(pipeerr[1]); -if(non_block) -if(virSetNonBlock(pipeerr[0]) == -1) -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(Failed to set non-blocking file descriptor flag)); - -if(virSetCloseExec(pipeerr[0]) == -1) -ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, -_(Failed to set close-on-exec file descriptor flag)); *errfd = pipeerr[0]; } *retpid = pid; @@ -160,23 +170,47 @@ /* child */ -if (pipeout[0] 0 close(pipeout[0]) 0) +/* Don't want to report errors against this accidentally, so + just discard it */ +conn = NULL; +/* Remove any error callback too, so errors in child now + get sent to stderr where they stand a fighting chance
Re: [libvirt] PATCH: 2/5: Fix signal handler race condition
This is the same patch from yesterday to fix the signal handler race condition. We block signals before doing a fork(), and then in the child reset all signal handlers before finally unblocking all signals. The parent will restore its original signal mask after fork. I've fixed the incorrect return code check on pthread_sigmask() and iterate from 1 instead of 0. I'll switch to pid_t later, since that'll involve changing all callers too. In the internal.h file I also #define pthread_sigmask to sigprocmask for scenarios where we don't have pthread - as per other usage. This exposed a bug in remote_protocol.c file where it was not including the config.h file, hence that change here too qemud/remote_protocol.c |1 qemud/remote_protocol.h |1 qemud/remote_protocol.x |1 src/internal.h |1 src/util.c | 55 +++- 5 files changed, 58 insertions(+), 1 deletion(-) Daniel diff -r 76da44084eee qemud/remote_protocol.c --- a/qemud/remote_protocol.c Tue Aug 12 22:10:07 2008 +0100 +++ b/qemud/remote_protocol.c Wed Aug 13 10:07:22 2008 +0100 @@ -4,6 +4,7 @@ */ #include remote_protocol.h +#include config.h #include internal.h bool_t diff -r 76da44084eee qemud/remote_protocol.h --- a/qemud/remote_protocol.h Tue Aug 12 22:10:07 2008 +0100 +++ b/qemud/remote_protocol.h Wed Aug 13 10:07:22 2008 +0100 @@ -13,6 +13,7 @@ extern C { #endif +#include config.h #include internal.h #define REMOTE_MESSAGE_MAX 262144 #define REMOTE_STRING_MAX 65536 diff -r 76da44084eee qemud/remote_protocol.x --- a/qemud/remote_protocol.x Tue Aug 12 22:10:07 2008 +0100 +++ b/qemud/remote_protocol.x Wed Aug 13 10:07:22 2008 +0100 @@ -36,6 +36,7 @@ * 'REMOTE_'. This makes names quite long. */ +%#include config.h %#include internal.h /*- Data types. -*/ diff -r 76da44084eee src/internal.h --- a/src/internal.hTue Aug 12 22:10:07 2008 +0100 +++ b/src/internal.hWed Aug 13 10:07:22 2008 +0100 @@ -22,6 +22,7 @@ #define pthread_mutex_destroy(lk) /*empty*/ #define pthread_mutex_lock(lk) /*empty*/ #define pthread_mutex_unlock(lk) /*empty*/ +#define pthread_sigmask(h, s, o) sigprocmask((h), (s), (o)) #endif /* The library itself is allowed to use deprecated functions / diff -r 76da44084eee src/util.c --- a/src/util.cTue Aug 12 22:10:07 2008 +0100 +++ b/src/util.cWed Aug 13 10:07:22 2008 +0100 @@ -37,6 +37,7 @@ #include sys/wait.h #endif #include string.h +#include signal.h #include c-ctype.h #ifdef HAVE_PATHS_H @@ -49,6 +50,10 @@ #include util.h #include memory.h #include util-lib.c + +#ifndef NSIG +# define NSIG 32 +#endif #ifndef MIN # define MIN(a, b) ((a) (b) ? (a) : (b)) @@ -102,9 +107,23 @@ _virExec(virConnectPtr conn, const char *const*argv, int *retpid, int infd, int *outfd, int *errfd, int non_block) { -int pid, null; +int pid, null, i; int pipeout[2] = {-1,-1}; int pipeerr[2] = {-1,-1}; +sigset_t oldmask, newmask; +struct sigaction sig_action; + +/* + * Need to block signals now, so that child process can safely + * kill off caller's signal handlers without a race. + */ +sigfillset(newmask); +if (pthread_sigmask(SIG_SETMASK, newmask, oldmask) != 0) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(cannot block signals: %s), +strerror(errno)); +return -1; +} if ((null = open(_PATH_DEVNULL, O_RDONLY)) 0) { ReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -164,6 +183,16 @@ close(pipeerr[1]); *errfd = pipeerr[0]; } + +/* Restore our original signal mask now child is safely + running */ +if (pthread_sigmask(SIG_SETMASK, oldmask, NULL) != 0) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(cannot unblock signals: %s), +strerror(errno)); +return -1; +} + *retpid = pid; return 0; } @@ -177,6 +206,30 @@ get sent to stderr where they stand a fighting chance of being seen / logged */ virSetErrorFunc(NULL, NULL); + +/* Clear out all signal handlers from parent so nothing + unexpected can happen in our child once we unblock + signals */ +sig_action.sa_handler = SIG_DFL; +sig_action.sa_flags = 0; +sigemptyset(sig_action.sa_mask); + +for (i = 1 ; i NSIG ; i++) +/* Only possible errors are EFAULT or EINVAL + The former wont happen, the latter we + expect, so no need to check return value */ +sigaction(i, sig_action, NULL); + +/* Unmask all signals in child, since we've no idea + what the caller's done with their signal mask + and don't want to propagate that to children */ +sigemptyset(newmask); +if (pthread_sigmask(SIG_SETMASK, newmask, NULL) != 0) { +ReportError(conn,
Re: [libvirt] PATCH: 3/5: Allow FD for stdout/err to be passed in
The contract for virExec() currently allows the caller to pass in a NULL for stdout/err parameters in which case the child will be connected to /dev/null, or they can pass in a pointer to an int, in which case the child will be connected to a pair of pipes, and the read end of the pipe is returned to the caller. The LXC driver will require a 3rd option - we want to pass in a existing file handler connected to a logfile. So this patch extends the semantics of these two parameters. If the stderr/out params are non-NULL, but are initialized to -1, then a pipe will be allocated. If they are = 0, then they are assumed to be existing FDs to dup onto the child's stdout/err. This change neccessitated updating all existing callers of virExec to make sure they initialize the parameters to -1 to maintain existing behaviour. openvz_driver.c |4 - qemu_driver.c |2 util.c | 120 3 files changed, 73 insertions(+), 53 deletions(-) Daniel diff -r 100b059a8488 src/openvz_driver.c --- a/src/openvz_driver.c Tue Aug 12 22:12:38 2008 +0100 +++ b/src/openvz_driver.c Tue Aug 12 22:12:42 2008 +0100 @@ -736,7 +736,7 @@ static int openvzListDomains(virConnectPtr conn, int *ids, int nids) { int got = 0; -int veid, pid, outfd, errfd; +int veid, pid, outfd = -1, errfd = -1; int ret; char buf[32]; char *endptr; @@ -772,7 +772,7 @@ static int openvzListDefinedDomains(virConnectPtr conn, char **const names, int nnames) { int got = 0; -int veid, pid, outfd, errfd, ret; +int veid, pid, outfd = -1, errfd = -1, ret; char vpsname[OPENVZ_NAME_MAX]; char buf[32]; char *endptr; diff -r 100b059a8488 src/qemu_driver.c --- a/src/qemu_driver.c Tue Aug 12 22:12:38 2008 +0100 +++ b/src/qemu_driver.c Tue Aug 12 22:12:42 2008 +0100 @@ -948,6 +948,8 @@ if (safewrite(vm-logfile, \n, 1) 0) qemudLog(QEMUD_WARN, _(Unable to write argv to logfile %d: %s\n), errno, strerror(errno)); + +vm-stdout_fd = vm-stderr_fd = -1; ret = virExecNonBlock(conn, argv, vm-pid, vm-stdin_fd, vm-stdout_fd, vm-stderr_fd); diff -r 100b059a8488 src/util.c --- a/src/util.cTue Aug 12 22:12:38 2008 +0100 +++ b/src/util.cTue Aug 12 22:12:42 2008 +0100 @@ -110,6 +110,7 @@ int pid, null, i; int pipeout[2] = {-1,-1}; int pipeerr[2] = {-1,-1}; +int childout = -1, childerr = -1; sigset_t oldmask, newmask; struct sigaction sig_action; @@ -132,39 +133,66 @@ goto cleanup; } -if ((outfd != NULL pipe(pipeout) 0) || -(errfd != NULL pipe(pipeerr) 0)) { -ReportError(conn, VIR_ERR_INTERNAL_ERROR, -_(cannot create pipe: %s), strerror(errno)); -goto cleanup; +if (outfd != NULL) { +if (*outfd == -1) { +if (pipe(pipeout) 0) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(cannot create pipe: %s), strerror(errno)); +goto cleanup; +} + +if (non_block +virSetNonBlock(pipeout[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set non-blocking file descriptor flag)); +goto cleanup; +} + +if (virSetCloseExec(pipeout[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set close-on-exec file descriptor flag)); +goto cleanup; +} + +childout = pipeout[1]; +} else { +childout = *outfd; +} +#ifndef ENABLE_DEBUG +} else { +childout = null; +#endif } -if (outfd) { -if(non_block - virSetNonBlock(pipeout[0]) == -1) { -ReportError(conn, VIR_ERR_INTERNAL_ERROR, -_(Failed to set non-blocking file descriptor flag)); -goto cleanup; +if (errfd != NULL) { +if (*errfd == -1) { +if (pipe(pipeerr) 0) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(cannot create pipe: %s), strerror(errno)); +goto cleanup; +} + +if (non_block +virSetNonBlock(pipeerr[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set non-blocking file descriptor flag)); +goto cleanup; +} + +if (virSetCloseExec(pipeerr[0]) == -1) { +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(Failed to set close-on-exec file descriptor flag)); +goto cleanup; +} + +childerr = pipeerr[1]; +} else { +childerr = *errfd; } - -
Re: [libvirt] PATCH: 4/5: Support daemonizing child env variables
Some of the existing usage of fork/exec in libvirt is done such that the child process is daemonized. In particular the libvirt_proxy and the auto-spawned libvirtd for qemu:///session. Since we want to switch these to use virExec, we need to suport a daemon mode. This patch removes the two alternate virExec and virExecNonBlock functions and renames the internal __virExec to virExec. It then gains a 'flags' parameter which can be used to specify non-blocking mode, or daemon mode. We also add the ability to pass in a list of environment variables which get passed on to execve(). We also now explicitly close all file handles. Although libvirt code is careful to set O_CLOSEXEC on all its file handles, in multi-threaded apps there is a race condition between opening a FD and setting O_CLOSEXEC. Furthermore, we can't guarentee that all applications using libvirt are careful to set O_CLOSEXEC. Leaking FDs to a child is a potential security risk and often causes SELinux AVCs to be raised. Thus explicitely closing all FDs is a important safety net. openvz_driver.c |4 +- qemu_driver.c |7 ++-- storage_backend.c |4 +- util.c| 78 +- util.h| 18 +--- 5 files changed, 71 insertions(+), 40 deletions(-) Daniel diff -r 28ddf9f5791c src/openvz_driver.c --- a/src/openvz_driver.c Tue Aug 12 22:12:42 2008 +0100 +++ b/src/openvz_driver.c Tue Aug 12 22:13:02 2008 +0100 @@ -742,7 +742,7 @@ char *endptr; const char *cmd[] = {VZLIST, -ovpsid, -H , NULL}; -ret = virExec(conn, cmd, pid, -1, outfd, errfd); +ret = virExec(conn, cmd, NULL, pid, -1, outfd, errfd, VIR_EXEC_NONE); if(ret == -1) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _(Could not exec %s), VZLIST); @@ -779,7 +779,7 @@ const char *cmd[] = {VZLIST, -ovpsid, -H, -S, NULL}; /* the -S options lists only stopped domains */ -ret = virExec(conn, cmd, pid, -1, outfd, errfd); +ret = virExec(conn, cmd, NULL, pid, -1, outfd, errfd, VIR_EXEC_NONE); if(ret == -1) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _(Could not exec %s), VZLIST); diff -r 28ddf9f5791c src/qemu_driver.c --- a/src/qemu_driver.c Tue Aug 12 22:12:42 2008 +0100 +++ b/src/qemu_driver.c Tue Aug 12 22:13:02 2008 +0100 @@ -951,8 +951,9 @@ vm-stdout_fd = vm-stderr_fd = -1; -ret = virExecNonBlock(conn, argv, vm-pid, - vm-stdin_fd, vm-stdout_fd, vm-stderr_fd); +ret = virExec(conn, argv, NULL, vm-pid, + vm-stdin_fd, vm-stdout_fd, vm-stderr_fd, + VIR_EXEC_NONBLOCK); if (ret == 0) { vm-def-id = driver-nextvmid++; vm-state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING; @@ -1199,7 +1200,7 @@ if (qemudBuildDnsmasqArgv(conn, network, argv) 0) return -1; -ret = virExecNonBlock(conn, argv, network-dnsmasqPid, -1, NULL, NULL); +ret = virExec(conn, argv, NULL, network-dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK); for (i = 0; argv[i]; i++) VIR_FREE(argv[i]); diff -r 28ddf9f5791c src/storage_backend.c --- a/src/storage_backend.c Tue Aug 12 22:12:42 2008 +0100 +++ b/src/storage_backend.c Tue Aug 12 22:13:02 2008 +0100 @@ -403,7 +403,7 @@ /* Run the program and capture its output */ -if (virExec(conn, prog, child, -1, fd, NULL) 0) { +if (virExec(conn, prog, NULL, child, -1, fd, NULL, VIR_EXEC_NONE) 0) { goto cleanup; } @@ -537,7 +537,7 @@ v[i] = NULL; /* Run the program and capture its output */ -if (virExec(conn, prog, child, -1, fd, NULL) 0) { +if (virExec(conn, prog, NULL, child, -1, fd, NULL, VIR_EXEC_NONE) 0) { goto cleanup; } diff -r 28ddf9f5791c src/util.c --- a/src/util.cTue Aug 12 22:12:42 2008 +0100 +++ b/src/util.cTue Aug 12 22:13:02 2008 +0100 @@ -103,11 +103,14 @@ return 0; } -static int -_virExec(virConnectPtr conn, - const char *const*argv, - int *retpid, int infd, int *outfd, int *errfd, int non_block) { -int pid, null, i; +int +virExec(virConnectPtr conn, +const char *const*argv, +const char *const*envp, +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}; int childout = -1, childerr = -1; @@ -141,7 +144,7 @@ goto cleanup; } -if (non_block +if ((flags VIR_EXEC_NONBLOCK) virSetNonBlock(pipeout[0]) == -1) { ReportError(conn, VIR_ERR_INTERNAL_ERROR, _(Failed to set non-blocking file descriptor flag)); @@ -172,7 +175,7 @@ goto cleanup; } -if (non_block +if ((flags VIR_EXEC_NONBLOCK)
Re: [libvirt] PATCH: 5/5: Make all code use virExec
This final patch switches over all places which do fork()/exec() to use the new enhanced virExec() code. A fair amount of code is deleted, but that's not the whole story - the new impl is more robust that most of the existing code we're deleting here. bridge.c | 51 +++- proxy_internal.c | 25 +--- qemu_conf.c | 168 ++ remote_internal.c | 88 4 files changed, 100 insertions(+), 232 deletions(-) Daniel diff -r 2591ebc40bd7 src/bridge.c --- a/src/bridge.c Tue Aug 12 15:29:29 2008 +0100 +++ b/src/bridge.c Tue Aug 12 15:33:42 2008 +0100 @@ -46,6 +46,7 @@ #include internal.h #include memory.h +#include util.h #define MAX_BRIDGE_ID 256 @@ -596,42 +597,6 @@ return brGetInetAddr(ctl, ifname, SIOCGIFNETMASK, addr, maxlen); } -static int -brctlSpawn(char * const *argv) -{ -pid_t pid, ret; -int status; -int null = -1; - -if ((null = open(_PATH_DEVNULL, O_RDONLY)) 0) -return errno; - -pid = fork(); -if (pid == -1) { -int saved_errno = errno; -close(null); -return saved_errno; -} - -if (pid == 0) { /* child */ -dup2(null, STDIN_FILENO); -dup2(null, STDOUT_FILENO); -dup2(null, STDERR_FILENO); -close(null); - -execvp(argv[0], argv); - -_exit (1); -} - -close(null); - -while ((ret = waitpid(pid, status, 0) == -1) errno == EINTR); -if (ret == -1) -return errno; - -return (WIFEXITED(status) WEXITSTATUS(status) == 0) ? 0 : EINVAL; -} /** * brSetForwardDelay: @@ -649,7 +614,7 @@ const char *bridge, int delay) { -char **argv; +const char **argv; int retval = ENOMEM; int n; char delayStr[30]; @@ -680,7 +645,10 @@ argv[n++] = NULL; -retval = brctlSpawn(argv); +if (virRun(NULL, argv, NULL) 0) +retval = errno; +else +retval = 0; error: if (argv) { @@ -709,7 +677,7 @@ const char *bridge, int enable) { -char **argv; +const char **argv; int retval = ENOMEM; int n; @@ -737,7 +705,10 @@ argv[n++] = NULL; -retval = brctlSpawn(argv); +if (virRun(NULL, argv, NULL) 0) +retval = errno; +else +retval = 0; error: if (argv) { diff -r 2591ebc40bd7 src/proxy_internal.c --- a/src/proxy_internal.c Tue Aug 12 15:29:29 2008 +0100 +++ b/src/proxy_internal.c Tue Aug 12 15:33:42 2008 +0100 @@ -162,6 +162,7 @@ { const char *proxyPath = virProxyFindServerPath(); int ret, pid, status; +const char *proxyarg[2]; if (!proxyPath) { fprintf(stderr, failed to find libvirt_proxy\n); @@ -171,27 +172,11 @@ if (debug) fprintf(stderr, Asking to launch %s\n, proxyPath); -/* Become a daemon */ -pid = fork(); -if (pid == 0) { -long open_max; -long i; +proxyarg[0] = proxyPath; +proxyarg[1] = NULL; -/* don't hold open fd opened from the client of the library */ -open_max = sysconf (_SC_OPEN_MAX); -for (i = 0; i open_max; i++) -fcntl (i, F_SETFD, FD_CLOEXEC); - -setsid(); -if (fork() == 0) { -execl(proxyPath, proxyPath, NULL); -fprintf(stderr, _(failed to exec %s\n), proxyPath); -} -/* - * calling exit() generate troubles for termination handlers - */ -_exit(0); -} +if (virExec(NULL, proxyarg, NULL, pid, -1, NULL, NULL, VIR_EXEC_DAEMON) 0) +fprintf(stderr, Failed to fork libvirt_proxy\n); /* * do a waitpid on the intermediate process to avoid zombies. diff -r 2591ebc40bd7 src/qemu_conf.c --- a/src/qemu_conf.c Tue Aug 12 15:29:29 2008 +0100 +++ b/src/qemu_conf.c Tue Aug 12 15:33:42 2008 +0100 @@ -397,116 +397,88 @@ int qemudExtractVersionInfo(const char *qemu, int *version, int *flags) { +const char *const qemuarg[] = { qemu, -help, NULL }; +const char *const qemuenv[] = { LANG=C, NULL }; pid_t child; -int newstdout[2]; +int newstdout = -1; +char help[8192]; /* Ought to be enough to hold QEMU help screen */ +int got = 0, ret = -1, status; +int major, minor, micro; +int ver; if (flags) *flags = 0; if (version) *version = 0; -if (pipe(newstdout) 0) { +if (virExec(NULL, qemuarg, qemuenv, child, +-1, newstdout, NULL, VIR_EXEC_NONE) 0) return -1; + + +while (got (sizeof(help)-1)) { +int len; +if ((len = read(newstdout, help+got, sizeof(help)-got-1)) = 0) { +if (!len) +break; +if (errno == EINTR) +continue; +goto cleanup2; +} +got += len; +} +help[got] = '\0'; + +if (sscanf(help, QEMU PC emulator version %d.%d.%d,
Re: [libvirt] [PATCH] Change disk type 'dos' to 'msdos'
On Tue, Aug 12, 2008 at 11:58:12PM -0400, Cole Robinson wrote: Daniel Veillard wrote: Fine by me, my only worry is that we are somehow breaking the storage XML format as a result, but I don't think this is widely used at this point (at least with MSDos) and best done earlier than later, Applied and commited, thanks ! Daniel Wait, I think there was a bit of miscommunication. I think Jim and Dan were recommending a different approach then the patch that was committed. The attached patch keeps the public facing formats the same, we just special case the dos format to use msdos if calling out to parted (this would require the current patch to be reverted though). Okay, I think I'm the one who misunderstood, so reverted and applied the new fix which only change at the parted call level. Sorry :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about html file in docs
On Wed, Aug 13, 2008 at 04:37:27PM +0900, Atsushi SAKAI wrote: Hi, Daniel If I run the make on libvirt, cvs diff outputs large diffs on html. This is because the difference of html and html.in is large. What kind of diff do you get ? The html output should only be dependant on the html.in and the .xsl stylesheets I think it should be fixed. One should see changes in .html only if the .in and .xsl changed so: 1/ you should not have any rebuild done on your CVS checkout unless you have timestamp problems on the checkout box 2/ if it were to be rebuilt it really should be the same as on the checkout So there is at least 2 problems on your checkouts, I wonder why Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about html file in docs
On Wed, Aug 13, 2008 at 09:35:04AM +0100, Daniel P. Berrange wrote: On Wed, Aug 13, 2008 at 03:34:33AM -0400, Daniel Veillard wrote: On Wed, Aug 13, 2008 at 09:56:00AM +0900, Atsushi SAKAI wrote: Hi, I think html files in docs directory are redundunt. It is because html file is created by html.in. May I remove these files? No Or are there any reason about staying these files? Because the web site is a CVS checkout of the docs/ subdir Could the cron script which updates the CVS checkout simply run 'make' in the docs/ directory ? It already has to run a couple of scripts to re-generate the NEWS ChangeLog.html files on every checkout Well I dislike the idea that the correctness of the website is dependant on the ability to rebuild libvirt on the box, this is RHEL4, autogen/configure may fail, NEWS and ChangeLog.html are not crucial, but I would like to be sure the site content stay okay even in case of troubles on the box. And really Atsushi should not see diffs like this, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 1/7: Removing state from lxc_vm_t
On Mon, Aug 11, 2008 at 12:25:52PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: This patch does some simple re-factoring of the way the TTYs and control socket are handled to reduce the amount of state stored in the lxc_vm_t structure, in preparation for the switchover to the generic domain handling APIs. ... diff -r 63b8398c302e src/lxc_driver.c --- a/src/lxc_driver.c Mon Jul 14 12:18:23 2008 +0100 +++ b/src/lxc_driver.c Tue Jul 15 11:55:48 2008 +0100 ... @@ -989,15 +896,18 @@ lxc_vm_t * vm) { int rc = -1; -lxc_vm_def_t *vmDef = vm-def; +int sockpair[2]; ... +if (lxcOpenTty(conn, parentTty, vm-def-tty, 1) 0) { goto cleanup; } /* open container tty */ -if (lxcSetupContainerTty(conn, (vm-containerTtyFd), (vm-containerTty)) 0) { +if (lxcOpenTty(conn, containerTty, containerTtyPath, 0) 0) { goto cleanup; } ... +if (0 != socketpair(PF_UNIX, SOCK_STREAM, 0, sockpair)) { ... cleanup: -close(vm-sockpair[LXC_PARENT_SOCKET]); -vm-sockpair[LXC_PARENT_SOCKET] = -1; -close(vm-sockpair[LXC_CONTAINER_SOCKET]); -vm-sockpair[LXC_CONTAINER_SOCKET] = -1; +close(sockpair[0]); +close(sockpair[1]); +VIR_FREE(containerTtyPath); return rc; } All looks fine except for the possibility that the cleanup code can close undefined sockpair[0,1]. The obvious fix is to initialize them to -1 and not close in that case. Yep, I've made that change committed this. Oh, and the new name, monitor (new struct member and local/param in several functions) would be more readable as monitor_fd. The struct member will be going away in later re-factoring so I've not changed this naming 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/7: Removing state from lxc_vm_t
On Mon, Aug 11, 2008 at 12:50:46PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: This patch does some simple re-factoring of the way the TTYs and control socket are handled to reduce the amount of state stored in the lxc_vm_t structure, in preparation for the switchover to the generic domain handling APIs. One more thing: ... diff -r 63b8398c302e src/lxc_container.c --- a/src/lxc_container.c Mon Jul 14 12:18:23 2008 +0100 +++ b/src/lxc_container.c Tue Jul 15 11:55:48 2008 +0100 ... -close(0); close(1); close(2); +/* Just in case someone forget to set FD_CLOEXEC, explicitly + * close all FDs before executing the container */ +open_max = sysconf (_SC_OPEN_MAX); +for (i = 0; i open_max; i++) +if (i != ttyfd) +close(i); Do you really need to close all file descriptors 2 ? I seem to recall that an application doing this caused trouble when it closed a file descriptor (opened via the shell that I was using for log output. This is important to ensuring no file descriptors are leaked into the container we run because that would be a potential security problem. In any case this code will be replaced by a call to virExec() by a later patch in this series. 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: 2/7: Re-arrange methods acros LXC source files
On Mon, Aug 11, 2008 at 07:16:20PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: The lxc_driver.c file contains quite a large amount of code, serving two reasonably well separated purposes. First there is the direct implemntation of each of the libvirt driver APIs. Second there is the code to spawn a container and a controller for forwarding I/O to/from the PTYs. This patch attempts to re-arrange the code across files to better reflect the split in functionality. The general idea is thus: - lxc_driver.c: implementation of the libvirt driver APIs - lxc_container.c: code for creating containers - lxc_controller.c: code for managing an active container So this entails the following re-arrangement: Most changes were straight function copies. In the few cases where there were nontrivial changes, they looked fine, though it was tedious to extract the diffs. Ok I've committed this patch too now. 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: 3/7:
On Tue, Aug 12, 2008 at 11:07:00AM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: diff -r 8093fb566748 src/lxc_conf.c diff -r 8093fb566748 src/lxc_conf.h --- a/src/lxc_conf.hFri Aug 01 14:47:33 2008 +0100 +++ b/src/lxc_conf.hTue Aug 05 12:13:24 2008 +0100 @@ -46,7 +46,6 @@ struct __lxc_net_def { int type; char *parentVeth; /* veth device in parent namespace */ -char *containerVeth;/* veth device in container namespace */ char *txName; /* bridge or network name */ lxc_net_def_t *next; @@ -87,11 +86,10 @@ struct __lxc_vm { int pid; int state; +int monitor; I like monitor_fd ;-) This struct goes away in the next patch. diff -r 8093fb566748 src/lxc_container.c --- a/src/lxc_container.c Fri Aug 01 14:47:33 2008 +0100 +++ b/src/lxc_container.c Tue Aug 05 12:13:24 2008 +0100 @@ -69,6 +69,8 @@ typedef struct __lxc_child_argv lxc_child_argv_t; struct __lxc_child_argv { lxc_vm_def_t *config; +int nveths; From the looks of all uses, this can be an unsigned type. Yes, I've updated this to be unsigned throughout. @@ -230,21 +231,21 @@ * * Returns 0 on success or nonzero in case of error */ -static int lxcContainerEnableInterfaces(const lxc_vm_def_t *def) +static int lxcContainerEnableInterfaces(int nveths, +char **veths) { -int rc = 0; -const lxc_net_def_t *net; +int rc = 0, i; Many style guides recommend against putting multiple ,-separated declarations on the same line... I've split this onto 2 declarations anyway, because 'i' can now be unsigned. @@ -337,12 +338,11 @@ int flags; int stacksize = getpagesize() * 4; char *stack, *stacktop; -lxc_child_argv_t args = { def, control, ttyPath }; +lxc_child_argv_t args = { def, nveths, veths, control, ttyPath }; If possible, it'd be nice to make the struct const. Doesn't help much, because then I have to cast-away the constness when passing it into clone(). @@ -379,8 +379,9 @@ char *stack; int childStatus; -if (features LXC_CONTAINER_FEATURE_NET) +if (features LXC_CONTAINER_FEATURE_NET) { flags |= CLONE_NEWNET; +} Since you're making a change like this, I suspect it's worth adding a sentence+example to HACKING saying that we prefer to use braces even when there's only one statement. Out of habit, I tend *not* to use braces in that case, but have no trouble adapting. Codifying it might help avoid a little churn. No, that's a bogus change - I don't like to have {} for single line statements - its just unneccessary noise. @@ -138,23 +198,46 @@ /* if active fd's, return if no events, else wait forever */ timeout = (numActive 0) ? 0 : -1; numEvents = epoll_wait(epollFd, epollEvent, 1, timeout); -if (0 numEvents) { -if (epollEvent.events EPOLLIN) { -curFdOff = epollEvent.data.u32; -if (!fdArray[curFdOff].active) { -fdArray[curFdOff].active = 1; -++numActive; +if (numEvents 0) { +if (epollEvent.data.fd == monitor) { +int fd = accept(monitor, NULL, 0); +if (client != -1 || /* Already connected, so kick new one out */ I presume you meant to insert client = fd; right after fd = ..., (or compare fd != -1, and then set client = fd after the }), since it looks like client must be defined after this if/else chain. Yes, there should have been a 'client = fd;' statement after the conditional. +kill(container, SIGTERM); +waitpid(container, NULL, 0); It might be worthwhile to detect kill or waitpid failure when rc == 0. Any more to the point, it should not be invoked at all when container == -1, because that kills off every process on your system except for init. Yes, that's happened to me several times while debugging this :-) + * container exits. + * + * Returns 0 on success or -1 in case of error + */ +static int lxcVMCleanup(virConnectPtr conn, +lxc_driver_t *driver, +lxc_vm_t * vm) +{ +int rc = -1; +int waitRc; +int childStatus = -1; + +while (((waitRc = waitpid(vm-pid, childStatus, 0)) == -1) + errno == EINTR); It's easier to recognize this as an empty loop if you give it a comment and put the semicolon on its own line: while (((waitRc = waitpid(vm-pid, childStatus, 0)) == -1) errno == EINTR) ; /* empty */ Made this change. +goto error; +} + +memset(addr, 0, sizeof(addr)); +addr.sun_family = AF_UNIX; +strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + +if (connect(fd, (struct sockaddr *) addr,
Re: [libvirt] [PATCH] Fix size reporting for disk pools without partitions
On Tue, Aug 12, 2008 at 11:58:23PM -0400, Cole Robinson wrote: The attached patch updates parthelper to print size information for a disk device if it doesn't have any allocated partitions. The current code starts by requesting the first partition, then iterating from there. But if there is no first partition, that whole info reporting thing never happens :) Seems to produce desired results for partitioned and unpartitioned devices. Ooh, nice one finding this bug. ACK 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] virt-install?
On Wed, Aug 13, 2008 at 5:43 PM, Daniel P. Berrange [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 04:59:35PM +0900, Jun Koi wrote: On Wed, Aug 13, 2008 at 4:43 PM, Daniel Veillard [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 04:35:00PM +0900, Jun Koi wrote: Hi, I got the libvirt source code (cvs), and compiled. However, I cannot find the virt-install anywhere. Is it included inside libvirt? No, a companion of virt-manager project see http://virt-manager.org/scmrepo.html to fetch the sources I got virtinstall, but there is no INSTALL file inside (should have one, or README should mention that). So how to install it?? It uses the standard python installation procedure As root python setup.py install Or as non-root python setup.py install --prefix=$HOME/usr export PYTHONPATH=$HOME/usr/lib/python2.5/site-packages Thanks! But that needs to be mentioned anyway in README/INSTALL, as not everybody is Python expert. Regards, Jun -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about html file in docs
On Wed, Aug 13, 2008 at 05:51:25AM -0400, Daniel Veillard wrote: On Wed, Aug 13, 2008 at 04:37:27PM +0900, Atsushi SAKAI wrote: Hi, Daniel If I run the make on libvirt, cvs diff outputs large diffs on html. This is because the difference of html and html.in is large. We investigated a bit with Chris Lalancette who is seeing the same problem What kind of diff do you get ? The html output should only be dependant on the html.in and the .xsl stylesheets Well apparently the XSL output is post-processed by xmlling --valid --format which then introduce a dependancy on the XHTML1 DTDs . If you don't have them installed locally you will get validation error messages when building in docs and the output will diverge. The simplest solution for you is probably to make sure you have XHTML1 DTDs installed in your local XML catalog, which on RHEL/Fedora is as simple as having the package xhtml1-dtds installed on your machine(s). I think it should be fixed. One should see changes in .html only if the .in and .xsl changed so: 1/ you should not have any rebuild done on your CVS checkout unless you have timestamp problems on the checkout box That is still a problem, I see some files being rebuilt on a fresh checkout there is some timestamp problems in CVS state I guess. 2/ if it were to be rebuilt it really should be the same as on the checkout unfortunately no because xsltproc output is post processed, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 4/7: Convert LXC to new domain APIs
On Tue, Aug 12, 2008 at 04:39:05PM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: The re-architecting of the LXC controller/container process relationship in the previous patch removed the last obstacle to switching over to the generic domain XML routines. So this patch switches the driver over. First the vast majority of lxc_conf.h/c is simply deleted - this is all redundant when using the domain_conf.h APIs. Then, all references to lxc_vm_t are changed to virDomainObj, and lxc_vm_def_t switches to virDomainDef. Finally the LXC driver registers its capabilities data. For this I have chosen an OS type of 'exe', since the 'operating system' we're running in the container is just any plain executable process. lxc_conf.c | 1052 +-- lxc_conf.h | 121 -- lxc_container.c | 23 - lxc_container.h |2 lxc_controller.c |4 lxc_controller.h |2 lxc_driver.c | 289 --- 7 files changed, 215 insertions(+), 1278 deletions(-) All looks fine. ACK. However, please note that it would have been a lot easier/quicker to review if you'd done the mechanical/automatable changes (i.e., the global substitutions like s/lxc_vm_t/virDomainObj/) separately from the others. If i'd separated out the substitution, then the intermediate state between the two patches would not be functional. I prefer to have the changes fully operational at each step so you can bisect change history. 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] check for XHTML1 DTDs availability
On Wed, Aug 13, 2008 at 08:10:36AM -0400, Daniel Veillard wrote: Well apparently the XSL output is post-processed by xmlling --valid --format which then introduce a dependancy on the XHTML1 DTDs . If you don't have them installed locally you will get validation error messages when building in docs and the output will diverge. The simplest solution for you is probably to make sure you have XHTML1 DTDs installed in your local XML catalog, which on RHEL/Fedora is as simple as having the package xhtml1-dtds installed on your machine(s). The patch enclosed adds detection for the XHTML1 dtds at runtime on the build machine, and should solve the problem of diverging docs, replace the XML validity warnings by a more general message and add the requirement when building the RPM. I guess this should solve the issue Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ Index: configure.in === RCS file: /data/cvs/libxen/configure.in,v retrieving revision 1.158 diff -u -r1.158 configure.in --- configure.in29 Jul 2008 08:42:56 - 1.158 +++ configure.in13 Aug 2008 12:38:02 - @@ -97,6 +97,7 @@ AC_PATH_PROG([MV], [mv], [/bin/mv]) AC_PATH_PROG([TAR], [tar], [/bin/tar]) AC_PATH_PROG([XMLLINT], [xmllint], [/usr/bin/xmllint]) +AC_PATH_PROG([XMLCATALOG], [xmlcatalog], [/usr/bin/xmlcatalog]) AC_PATH_PROG([XSLTPROC], [xsltproc], [/usr/bin/xsltproc]) AC_PROG_MKDIR_P Index: libvirt.spec.in === RCS file: /data/cvs/libxen/libvirt.spec.in,v retrieving revision 1.89 diff -u -r1.89 libvirt.spec.in --- libvirt.spec.in 8 Aug 2008 14:27:05 - 1.89 +++ libvirt.spec.in 13 Aug 2008 12:38:02 - @@ -72,6 +72,7 @@ BuildRequires: xen-devel %endif BuildRequires: libxml2-devel +BuildRequires: xhtml1-dtds BuildRequires: readline-devel BuildRequires: ncurses-devel BuildRequires: gettext Index: docs/Makefile.am === RCS file: /data/cvs/libxen/docs/Makefile.am,v retrieving revision 1.25 diff -u -r1.25 Makefile.am --- docs/Makefile.am28 Apr 2008 08:29:35 - 1.25 +++ docs/Makefile.am13 Aug 2008 12:38:02 - @@ -94,18 +94,22 @@ $(XSLTPROC) --stringparam pagename $$name --nonet --html $(top_srcdir)/docs/site.xsl $ $@ || (rm $@ exit 1) ; fi ) %.html: %.html.tmp - @(if [ -x $(XMLLINT) ] ; then \ + @(if [ -x $(XMLLINT) -a -x $(XMLCATALOG) ] ; then \ + if $(XMLCATALOG) /etc/xml/catalog http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; /dev/null ; then \ echo Validating $@ ; \ - $(XMLLINT) --nonet --format --valid $ $@ || : ; fi ); + $(XMLLINT) --nonet --format --valid $ $@ || : ; \ + else echo missing XHTML1 DTD ; fi ; fi ); $(srcdir)/html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in -@(if [ -x $(XSLTPROC) ] ; then \ echo Rebuilding the HTML pages from the XML API ; \ $(XSLTPROC) --nonet $(srcdir)/newapi.xsl libvirt-api.xml ; fi ) - -@(if [ -x $(XMLLINT) ] ; then \ + -@(if [ -x $(XMLLINT) -a -x $(XMLCATALOG) ] ; then \ + if $(XMLCATALOG) /etc/xml/catalog http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; /dev/null ; then \ echo Validating the resulting XHTML pages ; \ - $(XMLLINT) --nonet --valid --noout html/*.html ; fi ); + $(XMLLINT) --nonet --valid --noout html/*.html ; \ + else echo missing XHTML1 DTD ; fi ; fi ); libvirt-api.xml libvirt-refs.xml: apibuild.py \ $(srcdir)/../include/libvirt/*.h \ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] manage number of virtual CPUs
OpenVZ containers use all CPUs available in system by default. Limitations may be caused only by Linux kernel limitation. There is way to artificially limit number of CPUs. patch add cpu management functionality to OpenVZ driver. Index: openvz_conf.c === RCS file: /data/cvs/libvirt/src/openvz_conf.c,v retrieving revision 1.33 diff -u -p -r1.33 openvz_conf.c --- openvz_conf.c 5 Aug 2008 10:53:05 - 1.33 +++ openvz_conf.c 13 Aug 2008 12:49:58 - @@ -513,6 +513,7 @@ openvzGetVPSInfo(virConnectPtr conn) { struct openvz_vm **pnext; struct openvz_driver *driver; struct openvz_vm_def *vmdef; +char temp[124]; vm = NULL; driver = conn-privateData; @@ -569,6 +570,17 @@ openvzGetVPSInfo(virConnectPtr conn) { goto error; } +/*get VCPU*/ +ret = openvzReadConfigParam(veid, CPUS, temp, sizeof(temp)); +if (ret 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, +_(Cound not read config for container %d), veid); + goto error; +} else if (ret 0) { + vmdef-vcpus = strtoI(temp); +} + + (*pnext)-vmdef = vmdef; pnext = (*pnext)-next; } Index: openvz_driver.c === RCS file: /data/cvs/libvirt/src/openvz_driver.c,v retrieving revision 1.37 diff -u -p -r1.37 openvz_driver.c --- openvz_driver.c 5 Aug 2008 10:53:05 - 1.37 +++ openvz_driver.c 13 Aug 2008 12:49:58 - @@ -94,6 +94,9 @@ static int openvzDomainUndefine(virDomai static void cmdExecFree(char *cmdExec[]); static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); +static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type); +static int openvzDomainGetMaxVcpus(virDomainPtr dom); +static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus); struct openvz_driver ovz_driver; @@ -266,7 +269,7 @@ static int openvzDomainGetInfo(virDomain //info-cpuTime = //info-maxMem = vm-def-maxmem; //info-memory = vm-def-memory; -//info-nrVirtCpu = vm-def-vcpus; +info-nrVirtCpu = vm-vmdef-vcpus; return 0; } @@ -450,7 +453,6 @@ openvzDomainDefineXML(virConnectPtr conn goto exit; } -//TODO: set number virtual CPUs //TODO: set quota if (virRun(conn, (char **)prog, NULL) 0) { @@ -475,6 +477,14 @@ openvzDomainDefineXML(virConnectPtr conn goto exit; } +if (vmdef-vcpus 0) { +if (openvzDomainSetVcpus(dom, vmdef-vcpus) 0) { +openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _(Could not set number of virtual cpu)); + goto exit; +} +} + exit: cmdExecFree(prog); return dom; @@ -548,6 +558,15 @@ openvzDomainCreateLinux(virConnectPtr co dom = virGetDomain(conn, vm-vmdef-name, vm-vmdef-uuid); if (dom) dom-id = vm-vpsid; + +if (vmdef-vcpus 0) { +if (openvzDomainSetVcpus(dom, vmdef-vcpus) 0) { +openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _(Could not set number of virtual cpu)); + goto exit; +} +} + exit: cmdExecFree(progcreate); return dom; @@ -662,6 +681,52 @@ openvzDomainGetAutostart(virDomainPtr do return 0; } +static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) { +if (STRCASEEQ(type, openvz)) +return 4096; //OpenVZ has no limitation + +openvzError(conn, VIR_ERR_INVALID_ARG, + _(unknown type '%s'), type); +return -1; +} + + +static int openvzDomainGetMaxVcpus(virDomainPtr dom) { +return openvzGetMaxVCPUs(dom-conn, openvz); +} + +static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { +virConnectPtr conn= dom-conn; +struct openvz_driver *driver = (struct openvz_driver *) conn-privateData; +struct openvz_vm *vm = openvzFindVMByUUID(driver, dom-uuid); +char str_vcpus[32]; +const char *prog[] = { VZCTL, --quiet, set, vm-vmdef-name, + --cpus, str_vcpus, --save, NULL }; +snprintf(str_vcpus, 31, %d, nvcpus); +str_vcpus[31] = '\0'; + +if (nvcpus = 0) { +openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _(VCPUs should be = 1)); +return -1; +} + +if (!vm) { +openvzError(conn, VIR_ERR_INVALID_DOMAIN, + _(no domain with matching uuid)); +return -1; +} + +if (virRun(conn, (char **)prog, NULL) 0) { +openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _(Could not exec %s), VZCTL); +return -1; +} + +vm-vmdef-vcpus = nvcpus; +return 0; +} + static const char *openvzProbe(void) { #ifdef __linux__ @@ -884,7 +949,7 @@ static virDriver openvzDriver = { NULL, /* version */ NULL, /* hostname */ NULL, /* uri */ -NULL,
Re: [libvirt] [PATCH] check for XHTML1 DTDs availability
Daniel Veillard wrote: On Wed, Aug 13, 2008 at 08:10:36AM -0400, Daniel Veillard wrote: Well apparently the XSL output is post-processed by xmlling --valid --format which then introduce a dependancy on the XHTML1 DTDs . If you don't have them installed locally you will get validation error messages when building in docs and the output will diverge. The simplest solution for you is probably to make sure you have XHTML1 DTDs installed in your local XML catalog, which on RHEL/Fedora is as simple as having the package xhtml1-dtds installed on your machine(s). The patch enclosed adds detection for the XHTML1 dtds at runtime on the build machine, and should solve the problem of diverging docs, replace the XML validity warnings by a more general message and add the requirement when building the RPM. I guess this should solve the issue Yes, that did it for me. I like the warning message too; at least that gives someone a fighting chance of figuring out what to install if they do want to generate the docs for some reason. +1 Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: 0/4: Make LXC controller a separate binary
Currently libvirt is just forking off a child process to handle the LXC I/O controller. This works OK, but is kinda evil. This series of patches makes it into a fully-fledged exec()able binary. With a tiny bit more work, you could even use this to launch an LXC container standalone without needing libvirt - which could be useful for debugging during development at the very least. The other nice thing is that it makes a 'ps' listing more meaningful - you see a 'libvirt_lxc' process and the name of the container as a command line arg. 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: 2/4: Restructure sources files in Makefile.am
Over time we've added lots of general purpose source code files, as wel as making more of the existing ones conditionally compiled. We've done this by adding lots of #ifdef WITH_ macros across the various source files. This is becoming rather a minefield with soo many conditionals sprinkled throughout the code. With all the recent re-factoring we now have very good separation between generic code, and driver specific code - each typically being in separate files. Thus it is now practical to remove the vast majority of the macros and just do the conditional compilation via the Makefile.am. The patch is not entirely clear - the resulting Makefile.am is much easier to review. It is structured in two section, first we declare variables for groups of source code files - eg generic helper files, generic domain XML files, generic network XML files, and then per-driver files. # These files are not related to driver APIs. Simply generic # helper APIs for various purposes GENERIC_LIB_SOURCES = \ bridge.c bridge.h \ buf.c buf.h \ conf.c conf.h \ event.c event.h \ iptables.c iptables.h \ memory.c memory.h \ qparams.c qparams.h \ stats_linux.c stats_linux.h \ uuid.c uuid.h \ util.c util.h \ virterror.c \ xml.c xml.h # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ capabilities.c capabilities.h \ domain_conf.c domain_conf.h \ nodeinfo.h nodeinfo.c # Network driver generic impl APIs NETWORK_CONF_SOURCES =\ network_conf.c network_conf.h # Storage driver generic impl APIs STORAGE_CONF_SOURCES =\ storage_conf.h storage_conf.c # The remote RPC driver, covering domains, storage, networks, etc REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ remote_internal.c remote_internal.h \ ../qemud/remote_protocol.c \ ../qemud/remote_protocol.h \ socketcompat.h # Mock driver, covering domains, storage, networks, etc TEST_DRIVER_SOURCES = \ test.c test.h # Now the Hypervisor specific drivers XEN_DRIVER_SOURCES = \ proxy_internal.c proxy_internal.h \ sexpr.c sexpr.h \ xen_internal.c xen_internal.h \ xen_unified.c xen_unified.h \ xend_internal.c xend_internal.h \ xm_internal.c xm_internal.h \ xs_internal.c xs_internal.h In the second section we then build up the libvirt_la_SOURCES variable, conditionally adding the driver specific sources according to what the user asked 'configure' to build libvirt_la_SOURCES = \ driver.h\ hash.c hash.h \ internal.h \ libvirt.c \ $(GENERIC_LIB_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(STORAGE_CONF_SOURCES) # Drivers usable outside daemon context if WITH_TEST libvirt_la_SOURCES += $(TEST_DRIVER_SOURCES) endif if WITH_REMOTE libvirt_la_SOURCES += $(REMOTE_DRIVER_SOURCES) endif if WITH_XEN libvirt_la_SOURCES += $(XEN_DRIVER_SOURCES) endif I've cut this down - the real makefile covers all drivers. Finally we also add all the sources to EXTRA_DIST, so that if someone does 'make dist', they include all the source files regardless of what has been configured to build. In removing the unneeded WITH_XXX macros from header/sources I noticed that a bunch of our header files have #ifdef __cplusplus extern C { #endif With is irrelevant since we're not using C++ anywhere - indeed some of the files even
Re: [libvirt] PATCH: 1/5: Improved error reporting
Daniel P. Berrange wrote: There are several system calls in the virExec function for which we don't or can't report errors. This patch does a couple of things to improve the situation. It moves the code for setting non-block/close-exec to before the fork() so we can report errors for it. It removes the 'dom' and 'net' params from the ReportError function since we deprecated those long ago and all callers simply pass in NULL. It resets the 'virErrorHandler' callback to NULL in the child, so that errors raised will get reported to stderr instead of invoking a callback which is likely no longer valid in the child process. It reports failures to exec the binary and dup file descriptors. All errors in the child will end up on stderr, so they will at least be visible on the parent's console, or a logfile if one was setup for the child. Previously there would just be silent failure. Daniel Related question: is there any practical way to return error output from a virRun command in a libvirt error message? In testing some of the storage code I hit a few bugs where we improperly called out to another app, and the raised libvirt error had no output. I would have to manually run libvirtd and watch what output the commands dumped. I guess the other option would be to set up log files for the different storage operations similar to how qemu domain logfiles work, or maybe just a general libvirtd output log. - Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 3/4: Introduce libvirt_lxc binary
This patch is the important one, switching from an I/O controller which is simply fork'd off libvirtd, to a properly execable libvirt_lxc binary. The libvirtd LXC driver writes the config it wants to launch with to /var/run/libvirt/lxc/NAME.log and invokves libvirt_lxc --name NAME This then loads the config and spawns the container. It also expects a '--console FD' arg to provide a file descriptor for the master PTY to use for the container's stdin/out/err. Currently this is compulsory but we can make it optional in future and have the container stdin/out/err plumbe straight to the libvirt_lxc process' existing stdin/out/err. This would allow launching containers standalone. If networking is configured, then we also pass '--veth NAME' for each configured interface specifying the name of the veth that will be moved into the container's namespace. The libvirt_lxc process writes a PID file to /var/run/libvirt/lxc/NAME.pid and opens a UNIX socket in the same dir - this is how libvirtd discovers when the container dies. a/src/lxc_controller.h | 41 -- src/Makefile.am| 26 +++ src/domain_conf.c |6 src/lxc_conf.c |8 - src/lxc_conf.h |4 src/lxc_controller.c | 320 +++-- src/lxc_driver.c | 191 +++-- 7 files changed, 366 insertions(+), 230 deletions(-) Daniel diff -r 1cf789924625 src/Makefile.am --- a/src/Makefile.am Tue Aug 12 22:21:48 2008 +0100 +++ b/src/Makefile.am Tue Aug 12 22:22:37 2008 +0100 @@ -88,8 +88,13 @@ LXC_DRIVER_SOURCES = \ lxc_conf.c lxc_conf.h \ lxc_container.c lxc_container.h \ - lxc_controller.c lxc_controller.h \ lxc_driver.c lxc_driver.h \ + veth.c veth.h + +LXC_CONTROLLER_SOURCES = \ + lxc_conf.c lxc_conf.h \ + lxc_container.c lxc_container.h \ + lxc_controller.c\ veth.c veth.h OPENVZ_DRIVER_SOURCES =\ @@ -272,9 +277,11 @@ rm -f $@ mv [EMAIL PROTECTED] $@ +libexec_PROGRAMS = + if WITH_STORAGE_DISK if WITH_LIBVIRTD -libexec_PROGRAMS = libvirt_parthelper +libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) @@ -284,6 +291,21 @@ endif EXTRA_DIST += $(STORAGE_HELPER_DISK_SOURCES) + +if WITH_LXC +if WITH_LIBVIRTD +libexec_PROGRAMS += libvirt_lxc + +libvirt_lxc_SOURCES = \ + $(LXC_CONTROLLER_SOURCES) \ + $(GENERIC_LIB_SOURCES) \ + $(DOMAIN_CONF_SOURCES) +libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_lxc_LDADD = $(LIBXML_LIBS) ../gnulib/lib/libgnu.la +libvirt_lxc_CFLAGS = $(LIBPARTED_CFLAGS) +endif +endif +EXTRA_DIST += $(LXC_CONTROLLER_SOURCES) # Create the /var/cache/libvirt directory when installing. install-exec-local: diff -r 1cf789924625 src/domain_conf.c --- a/src/domain_conf.c Tue Aug 12 22:21:48 2008 +0100 +++ b/src/domain_conf.c Tue Aug 12 22:22:37 2008 +0100 @@ -1109,13 +1109,11 @@ break; case VIR_DOMAIN_CHR_TYPE_PTY: -/* @path attribute is an output only property - pty is auto-allocted */ -break; - case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PIPE: -if (path == NULL) { +if (path == NULL +def-type != VIR_DOMAIN_CHR_TYPE_PTY) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(Missing source path attribute for char device)); goto error; diff -r 1cf789924625 src/lxc_conf.c --- a/src/lxc_conf.cTue Aug 12 22:21:48 2008 +0100 +++ b/src/lxc_conf.cTue Aug 12 22:22:37 2008 +0100 @@ -71,7 +71,7 @@ exe, utsname.machine, sizeof(int) == 4 ? 32 : 8, - NULL, + BINDIR /libvirt_lxc, NULL, 0, NULL)) == NULL) @@ -94,11 +94,11 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) { /* Set the container configuration directory */ -if ((driver-configDir = strdup(SYSCONF_DIR /libvirt/lxc)) == NULL) +if ((driver-configDir = strdup(LXC_CONFIG_DIR)) == NULL) goto no_memory; -if ((driver-stateDir = strdup(LOCAL_STATE_DIR
Re: [libvirt] PATCH: 4/4: Add pivot_root support to container
This isn't really related to the others, except for the fact that is part of the LXC driver. This is a re-post of the patch I did for adding support for pivot_root() in the container. This allows the entire container FS to be separated from the parent OS. As noted previously this is not currently secure, since Linux has not yet gained device namespace support, but is a stepping stone toward the final solution we'll need. Since the last posting I've added the explicit chmod() suggested by Jim, and the various other bug fixes lxc_container.c | 265 +--- util.c | 12 +- 2 files changed, 237 insertions(+), 40 deletions(-) Daniel diff -r 17f02fec7fe8 src/lxc_container.c --- a/src/lxc_container.c Wed Aug 13 14:39:28 2008 +0100 +++ b/src/lxc_container.c Wed Aug 13 14:39:34 2008 +0100 @@ -1,10 +1,12 @@ /* * Copyright IBM Corp. 2008 + * Copyright Red Hat 2008 * * lxc_container.c: file description * * Authors: * David L. Leskovec dlesko at linux.vnet.ibm.com + * Daniel P. Berrange [EMAIL PROTECTED] * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,10 +28,18 @@ #include fcntl.h #include limits.h #include stdlib.h +#include stdio.h #include sys/ioctl.h #include sys/mount.h #include sys/wait.h #include unistd.h +#include mntent.h + +/* Yes, we want linux private one, for _syscall2() macro */ +#include linux/unistd.h + +/* For MS_MOVE */ +#include linux/fs.h #include lxc_container.h #include util.h @@ -103,23 +113,15 @@ * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetStdio(int control, const char *ttyPath) +static int lxcContainerSetStdio(int control, int ttyfd) { int rc = -1; -int ttyfd; int open_max, i; if (setsid() 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _(setsid failed: %s), strerror(errno)); -goto error_out; -} - -ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); -if (ttyfd 0) { -lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _(open(%s) failed: %s), ttyPath, strerror(errno)); -goto error_out; +goto cleanup; } if (ioctl(ttyfd, TIOCSCTTY, NULL) 0) { @@ -156,9 +158,6 @@ rc = 0; cleanup: -close(ttyfd); - -error_out: return rc; } @@ -221,6 +220,7 @@ return 0; } + /** * lxcEnableInterfaces: * @vm: Pointer to vm structure @@ -251,6 +251,20 @@ return rc; } + +//_syscall2(int, pivot_root, char *, newroot, const char *, oldroot) +extern int pivot_root(const char * new_root,const char * put_old); + +static int lxcContainerChildMountSort(const void *a, const void *b) +{ + const char **sa = (const char**)a; + const char **sb = (const char**)b; + + /* Delibrately reversed args - we need to unmount deepest + children first */ + return strcmp(*sb, *sa); +} + /** * lxcChild: * @argv: Pointer to container arguments @@ -268,8 +282,8 @@ int rc = -1; lxc_child_argv_t *argv = data; virDomainDefPtr vmDef = argv-config; -virDomainFSDefPtr curMount; -int i; +virDomainFSDefPtr tmp, root = NULL; +int ttyfd, i; if (NULL == vmDef) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -277,37 +291,222 @@ return -1; } +#if 0 +ttyfd = open(argv-ttyPath, O_RDWR|O_NOCTTY); +if (ttyfd 0) { +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _(open(%s) failed: %s), argv-ttyPath, strerror(errno)); +return -1; +} +#endif + /* handle the bind mounts first before doing anything else that may */ /* then access those mounted dirs */ -curMount = vmDef-fss; -for (i = 0; curMount; curMount = curMount-next) { -// XXX fix -if (curMount-type != VIR_DOMAIN_FS_TYPE_MOUNT) +for (tmp = vmDef-fss; tmp !root; tmp = tmp-next) { +if (tmp-type != VIR_DOMAIN_FS_TYPE_MOUNT) continue; -rc = mount(curMount-src, - curMount-dst, - NULL, - MS_BIND, - NULL); -if (0 != rc) { +if (STREQ(tmp-dst, /)) +root = tmp; +} + +if (root) { +char *oldroot; +struct mntent *mntent; +char **mounts = NULL; +int nmounts = 0; +FILE *procmnt; +const struct { +int maj; +int min; +mode_t mode; +const char *path; +} devs[] = { +{ 1, 3, 0666, /dev/null }, +{ 1, 5, 0666, /dev/zero }, +{ 1, 7, 0666, /dev/full }, +{ 5, 1, 0600, /dev/console }, +{ 1, 8, 0666, /dev/random }, +{ 1, 9, 0666, /dev/urandom }, +}; + +/* Got a FS mapped to /, we're going the pivot_root + approach to do a better-chroot-than-chroot */ + +/* this is
Re: [libvirt] PATCH: 1/5: Improved error reporting
On Wed, Aug 13, 2008 at 10:21:59AM -0400, Cole Robinson wrote: Daniel P. Berrange wrote: There are several system calls in the virExec function for which we don't or can't report errors. This patch does a couple of things to improve the situation. It moves the code for setting non-block/close-exec to before the fork() so we can report errors for it. It removes the 'dom' and 'net' params from the ReportError function since we deprecated those long ago and all callers simply pass in NULL. It resets the 'virErrorHandler' callback to NULL in the child, so that errors raised will get reported to stderr instead of invoking a callback which is likely no longer valid in the child process. It reports failures to exec the binary and dup file descriptors. All errors in the child will end up on stderr, so they will at least be visible on the parent's console, or a logfile if one was setup for the child. Previously there would just be silent failure. Daniel Related question: is there any practical way to return error output from a virRun command in a libvirt error message? In testing some of the storage code I hit a few bugs where we improperly called out to another app, and the raised libvirt error had no output. I would have to manually run libvirtd and watch what output the commands dumped. Yes, I think we'd want to have virRun call virExec() setting up a pair of pipes for stderr/out. virRun would then read data from those pipes and stick into a string. The string could be returned to the caller, or just included in a libvirt error raised upon failure with non-zero exit status. I guess the other option would be to set up log files for the different storage operations similar to how qemu domain logfiles work, or maybe just a general libvirtd output log. I think we need to either feed it back to the caller directly, or raise an error. 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
[libvirt] [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
Hey, Here's a patch that allows libvirt created guests to use KVM's recent GSO support in order to increase the throughput achieved with virtio_net network interfaces. We shouldn't apply this yet - we need the kernel and kvm TUNGETIFF patches to be applied first - so this is just intended as an RFC. See also: http://marc.info/?l=linux-netdevm=121863813904363 and: http://marc.info/?l=kvmm=121863857305255 Cheers, Mark. Subject: [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds IFF_VNET_HDR is a tun/tap flag that allows you to send and receive large (i.e. GSO) packets and packets with partial checksums. Setting the flag means that every packet is proceeded by the same header which virtio uses to communicate GSO/csum metadata. By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net. However, we need to be careful to only set the flag when a) KVM has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl. Signed-off-by: Mark McLoughlin [EMAIL PROTECTED] Index: libvirt/src/bridge.c === --- libvirt.orig/src/bridge.c 2008-08-13 15:40:34.0 +0100 +++ libvirt/src/bridge.c2008-08-13 15:40:53.0 +0100 @@ -275,10 +275,52 @@ #endif /** + * brProbeVnetHdr: + * @tapfd: a tun/tap file descriptor + * + * Check whether it is safe to enable the IFF_VNET_HDR flag on the + * tap interface. + * + * Setting IFF_VNET_HDR enables KVM's virtio_net driver to allow + * guests to pass larger (GSO) packets, with partial checksums, to + * the host. This greatly increases the achievable throughput. + * + * It is only useful to enable this when we're setting up a virtio + * interface. And it is only *safe* to enable it when we know for + * sure that a) qemu has support for IFF_VNET_HDR and b) the running + * kernel implements the TUNGETIFF ioctl(), which qemu needs to query + * the supplied tapfd. + * + * Returns 0 in case of success or an errno code in case of failure. + */ +static int +brProbeVnetHdr(int tapfd) +{ +#if defined(IFF_VNET_HDR) defined(TUNGETFEATURES) defined(TUNGETIFF) +unsigned int features; +struct ifreq dummy; + +if (ioctl(tapfd, TUNGETFEATURES, features) != 0) +return 0; + +if (!(features IFF_VNET_HDR)) +return 0; + +if (ioctl(tapfd, TUNGETIFF, dummy) != -1 || errno != EBADFD) +return 0; + +return 1; +#else +return 0; +#endif +} + +/** * brAddTap: * @ctl: bridge control pointer * @bridge: the bridge name * @ifname: the interface name (or name template) + * @vnet_hdr: whether to try enabling iFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device * * This function creates a new tap device on a bridge. @ifname can be either @@ -292,6 +334,7 @@ brAddTap(brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd) { int id, subst, fd; @@ -307,6 +350,9 @@ if ((fd = open(/dev/net/tun, O_RDWR)) 0) return errno; +if (vnet_hdr) +vnet_hdr = brProbeVnetHdr(fd); + do { struct ifreq try; int len; @@ -315,6 +361,11 @@ try.ifr_flags = IFF_TAP|IFF_NO_PI; +#ifdef IFF_VNET_HDR +if (vnet_hdr) +try.ifr_flags |= IFF_VNET_HDR; +#endif + if (subst) { len = snprintf(try.ifr_name, BR_IFNAME_MAXLEN, *ifname, id); if (len = BR_IFNAME_MAXLEN) { Index: libvirt/src/bridge.h === --- libvirt.orig/src/bridge.h 2008-08-13 15:40:34.0 +0100 +++ libvirt/src/bridge.h2008-08-13 15:40:53.0 +0100 @@ -61,6 +61,7 @@ int brAddTap(brControl *ctl, const char *bridge, char **ifname, + int vnet_hdr, int *tapfd); int brSetInterfaceUp(brControl *ctl, Index: libvirt/src/qemu_conf.c === --- libvirt.orig/src/qemu_conf.c2008-08-13 15:40:34.0 +0100 +++ libvirt/src/qemu_conf.c 2008-08-13 15:41:15.0 +0100 @@ -474,6 +474,8 @@ *flags |= QEMUD_CMD_FLAG_DRIVE; if (strstr(help, boot=on)) *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; +if (strstr(help, IFF_VNET_HDR)) +*flags |= QEMUD_CMD_FLAG_VNET_HDR; if (ver = 9000) *flags |= QEMUD_CMD_FLAG_VNC_COLON; } @@ -545,7 +547,8 @@ int **tapfds, int *ntapfds, virDomainNetDefPtr net, - int vlan) + int vlan, + int vnet_hdr) { virNetworkObjPtr network = NULL; char
Re: [libvirt] [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
On Wed, Aug 13, 2008 at 03:53:27PM +0100, Mark McLoughlin wrote: By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net. However, we need to be careful to only set the flag when a) KVM has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl. This suggests to me that QEMU should be a the one enabling this flag, not libvirt. Its going to need todo this anyway for the scenario where QEMU allocates the TAP fd, so it might as well enable it for pre-allocated TAP fds passed in from external apps. 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] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
On Wed, 2008-08-13 at 16:01 +0100, Daniel P. Berrange wrote: On Wed, Aug 13, 2008 at 03:53:27PM +0100, Mark McLoughlin wrote: By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net. However, we need to be careful to only set the flag when a) KVM has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl. This suggests to me that QEMU should be a the one enabling this flag, not libvirt. It has to be done before adding it to the bridge and can't be changed afterwards. (Which perhaps suggests the kernel API is a little dumb, but ...) Its going to need todo this anyway for the scenario where QEMU allocates the TAP fd, so it might as well enable it for pre-allocated TAP fds passed in from external apps. Yeah, it already does it for the fds it allocates. Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
On Wed, 2008-08-13 at 15:53 +0100, Mark McLoughlin wrote: +net-model !strcmp(net-model, virtio)) Should be: +net-model STREQ(net-model, virtio)) Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/4: Restructure sources files in Makefile.am
On Wed, Aug 13, 2008 at 03:17:09PM +0100, Daniel P. Berrange wrote: Over time we've added lots of general purpose source code files, as wel as making more of the existing ones conditionally compiled. We've done this by adding lots of #ifdef WITH_ macros across the various source files. This is becoming rather a minefield with soo many conditionals sprinkled throughout the code. With all the recent re-factoring we now have very good separation between generic code, and driver specific code - each typically being in separate files. Thus it is now practical to remove the vast majority of the macros and just do the conditional compilation via the Makefile.am. The patch is not entirely clear - the resulting Makefile.am is much easier to review. It is structured in two section, first we declare variables for groups of source code files - eg generic helper files, generic domain XML files, generic network XML files, and then per-driver files. In general this sounds good, I don't like automake conditionals too much, but in that case yes, that's logical. [...] In removing the unneeded WITH_XXX macros from header/sources I noticed that a bunch of our header files have #ifdef __cplusplus extern C { #endif With is irrelevant since we're not using C++ anywhere - indeed some of the files even had the corresponding '}' missing, so clearly this is never actually used during compilation. Removing this fixes bogus indentation in some of the header files Well it should be kept in the public headers, but right internally it's just remains of cut and paste. I've tested this all by running configure --without-XXX for each hypervisor driver in turn, and it seemed to work in all cases. The only thing I didn't test is MinGW. I think I really need to add a MinGW based build to the nightly build system, so we can sanity check that nightly --- a/configure.inTue Aug 12 22:21:25 2008 +0100 +++ b/configure.inTue Aug 12 22:21:47 2008 +0100 @@ -241,27 +241,35 @@ LIBVIRT_FEATURES= WITH_XEN=0 -if test $with_openvz = yes ; then +if test $with_openvz = yes; then hum, unrelated :-) -if test $with_qemu = yes ; then +if test $with_qemu = yes -o $with_lxc = yes ; then AC_CHECK_HEADERS([linux/param.h linux/sockios.h linux/if_bridge.h linux/if_tun.h],, AC_MSG_ERROR([You must install kernel-headers in order to compile libvirt])) fi oh, good point ! [...] +if WITH_REMOTE +libvirt_la_SOURCES += $(REMOTE_DRIVER_SOURCES) +endif I'm wondering about += portability, but I think this is widely used and really clarifies the resulting Makefile.am the other solution libvirt_la_SOURCES = if WITH_REMOTE $(REMOTE_DRIVER_SOURCES) endif if WITH_XEN might be a bit more portable but messes the structure, No other comment, everything else is small bugs, the cleanup of headers and reindent, Fine by me, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds
On Wed, Aug 13, 2008 at 04:14:10PM +0100, Mark McLoughlin wrote: On Wed, 2008-08-13 at 16:01 +0100, Daniel P. Berrange wrote: On Wed, Aug 13, 2008 at 03:53:27PM +0100, Mark McLoughlin wrote: By enabling this flag on the tap fds we create, we greatly increase the achievable throughput with virtio_net. However, we need to be careful to only set the flag when a) KVM has support for this ABI and b) the value of the flag is queryable using the TUNGETIFF ioctl. This suggests to me that QEMU should be a the one enabling this flag, not libvirt. It has to be done before adding it to the bridge and can't be changed afterwards. Urgh, that's not nice :-( (Which perhaps suggests the kernel API is a little dumb, but ...) expletive deleted If we have todo this in libvirt, then the patch looks like a reasonable way todo it, but be nice if it wasn't our responsibility 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
[libvirt] [PATCH] fix minor typo
Fix a typo in the message for the 'dump' command in virsh. Signed-off-by: John Levon [EMAIL PROTECTED] Index: src/virsh.c === RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.157 diff -u -r1.157 virsh.c --- src/virsh.c 22 Jul 2008 16:12:01 - 1.157 +++ src/virsh.c 13 Aug 2008 15:50:09 - @@ -1320,7 +1320,7 @@ return FALSE; if (virDomainCoreDump(dom, to, 0) == 0) { -vshPrint(ctl, _(Domain %s dumpd to %s\n), name, to); +vshPrint(ctl, _(Domain %s dumped to %s\n), name, to); } else { vshError(ctl, FALSE, _(Failed to core dump domain %s to %s), name, to); -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tuesday 12 August 2008 5:57:19 am James Morris wrote: On Tue, 12 Aug 2008, Russell Coker wrote: One thing that should be noted is the labelled network benefits. If you had several groups of virtual servers running at different levels and wanted to prevent information leaks then having SE Linux contexts and labelled networking could make things a little easier. I have had some real challenges in managing firewall rules for Xen servers. My general practice is to try and make sure that there is no real need for firewalls between hosts on the same hardware (not that I want it this way - it's what technical and management issues force me to). So for example if I have an ISP Xen server running virtual machines for a number of organisations I make sure that they are either all within a similar trust boundary (IE affiliated groups) or all mutually untrusting (IE other IP addresses in the same net-block are treated the same as random hosts on the net). Thanks for the insights -- we expect to address the virtual networking aspect in some way. I think we could do some pretty cool things here with the new, well 2.6.25 new, network ingress/egress controls and restricting VM instances to specific interfaces and/or networks. However, we would need to settle the basic VM label management issues first. -- paul moore linux @ hp -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 3/4: Introduce libvirt_lxc binary
DB This patch is the important one, switching from an I/O controller DB which is simply fork'd off libvirtd, to a properly execable DB libvirt_lxc binary. This works for me. The network interfaces aren't cleaned up properly on destroy, and I get an error from the kernel about a reference count on the fake 'lo' interface for the container. However, I get that some of the time in the original code as well, so I don't see any reason holding this stuff up any further, and I will see about tracking that down. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: [EMAIL PROTECTED] pgp4Xn10o3VbP.pgp Description: PGP signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix serial console telnet protocol support
With e.g.: serial type='tcp' source mode='bind' host='127.0.0.1' service=''/ protocol type='telnet'/ target port='0'/ /serial You currently get: Unknown option: listen qemu: could not open serial device 'telnet:127.0.0.1:,listen' With the telnet protocol, qemu expects server as an option rather than listen. Signed-off-by: Mark McLoughlin [EMAIL PROTECTED] Index: libvirt/src/qemu_conf.c === --- libvirt.orig/src/qemu_conf.c2008-08-13 18:14:34.0 +0100 +++ libvirt/src/qemu_conf.c 2008-08-13 18:35:20.0 +0100 @@ -685,12 +685,19 @@ break; case VIR_DOMAIN_CHR_TYPE_TCP: -if (snprintf(buf, buflen, %s:%s:%s%s, - dev-data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET ? telnet : tcp, - dev-data.tcp.host, - dev-data.tcp.service, - dev-data.tcp.listen ? ,listen : ) = buflen) -return -1; +if (dev-data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) { +if (snprintf(buf, buflen, telnet:%s:%s%s, + dev-data.tcp.host, + dev-data.tcp.service, + dev-data.tcp.listen ? ,server : ) = buflen) +return -1; +} else { +if (snprintf(buf, buflen, tcp:%s:%s%s, + dev-data.tcp.host, + dev-data.tcp.service, + dev-data.tcp.listen ? ,listen : ) = buflen) +return -1; +} break; case VIR_DOMAIN_CHR_TYPE_UNIX: -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 4/4: Add pivot_root support to container
DB +#if 0 DB +ttyfd = open(argv-ttyPath, O_RDWR|O_NOCTTY); DB +if (ttyfd 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(open(%s) failed: %s), argv-ttyPath, strerror(errno)); DB +return -1; DB +} DB +#endif Should this bit be removed? DB +if (root) { DB +char *oldroot; DB +struct mntent *mntent; DB +char **mounts = NULL; DB +int nmounts = 0; DB +FILE *procmnt; DB +const struct { DB +int maj; DB +int min; DB +mode_t mode; DB +const char *path; DB +} devs[] = { DB +{ 1, 3, 0666, /dev/null }, DB +{ 1, 5, 0666, /dev/zero }, DB +{ 1, 7, 0666, /dev/full }, DB +{ 5, 1, 0600, /dev/console }, DB +{ 1, 8, 0666, /dev/random }, DB +{ 1, 9, 0666, /dev/urandom }, DB +}; DB + DB +/* Got a FS mapped to /, we're going the pivot_root DB + approach to do a better-chroot-than-chroot */ DB + DB +/* this is based on this thread http://lkml.org/lkml/2008/3/5/29 */ DB + DB +/* First step is to ensure the new root itself is DB + a mount point */ DB +if (mount(root-src, root-src, NULL, MS_BIND, NULL) 0) { DB lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB - _(failed to mount %s at %s for container: %s), DB - curMount-src, curMount-dst, strerror(errno)); DB + _(failed to bind new root %s: %s), DB + root-src, strerror(errno)); DB +return -1; DB +} DB + DB +if (asprintf(oldroot, %s/.oldroot, root-src) 0) { DB +lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); DB +return -1; DB +} DB + DB +if (virFileMakePath(oldroot) 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to create %s: %s), DB + oldroot, strerror(errno)); DB +return -1; DB +} DB + DB +/* The old root directory will live at /.oldroot after DB + * this and will soon be unmounted completely */ DB +if (pivot_root(root-src, oldroot) 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to pivot root %s to %s: %s), DB + oldroot, root-src, strerror(errno)); DB +return -1; DB +} DB + DB +/* CWD is undefined after pivot_root, so go to / */ DB +if (chdir(/) 0) { DB +return -1; DB +} DB + DB +if (virFileMakePath(/proc) 0 || DB +mount(none, /proc, proc, 0, NULL) 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to mount /proc for container: %s), DB + strerror(errno)); DB +return -1; DB +} DB +if (virFileMakePath(/dev) 0 || DB +mount(none, /dev, tmpfs, 0, NULL) 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to mount /dev tmpfs for container: %s), DB + strerror(errno)); DB +return -1; DB +} DB +/* Move old devpts into container, since we have to DB + connect to the master ptmx which was opened in DB + the parent. DB + XXX This sucks, we need to figure out how to get our DB + own private devpts for isolation DB +*/ DB +if (virFileMakePath(/dev/pts) 0 || DB +mount(/.oldroot/dev/pts, /dev/pts, NULL, DB + MS_MOVE, NULL) 0) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to move /dev/pts into container: %s), DB + strerror(errno)); DB +return -1; DB +} DB + DB +/* Populate /dev/ with a few important bits */ DB +for (i = 0 ; i ARRAY_CARDINALITY(devs) ; i++) { DB +dev_t dev = makedev(devs[i].maj, devs[i].min); DB +if (mknod(devs[i].path, 0, dev) 0 || DB +chmod(devs[i].path, devs[i].mode)) { DB +lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB + _(failed to make device %s: %s), DB + devs[i].path, strerror(errno)); DB +return -1; DB +} DB +} DB + DB +/* Pull in rest of container's mounts */ DB +for (tmp = vmDef-fss; tmp; tmp = tmp-next) { DB +char *src; DB +if (STREQ(tmp-dst, /)) DB +continue; DB +// XXX fix DB +if (tmp-type != VIR_DOMAIN_FS_TYPE_MOUNT) DB +continue; DB + DB +if (asprintf(src, /.oldroot/%s, tmp-src) 0) DB +return -1; DB + DB +if (virFileMakePath(tmp-dst) 0 || DB +mount(src, tmp-dst,
[libvirt] Reusing dumpxml output
I would like to reuse configurations that have been edited using the libvirt api. For example by attaching interfaces. Now the output that dumpxml generates, is very 'specific' to an active configuration. I am not able to use a stored configuration from an active domain to redefine it after it has been undefined. Is there a trick to do so? Or should one really strip all 'live' attributes first? Stefan -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] check for XHTML1 DTDs availability
Hi, Daniel I need to set http_proxy on my environment. (Sorry, since I need not set http_proxy since I use wget and cvs only) But still stays large diffs on html 6 files. drvqemu.html formatnetwork.html formatstorage.html intro.html news.html remote.html Thanks Atsushi SAKAI Daniel Veillard [EMAIL PROTECTED] wrote: On Wed, Aug 13, 2008 at 08:10:36AM -0400, Daniel Veillard wrote: Well apparently the XSL output is post-processed by xmlling --valid --format which then introduce a dependancy on the XHTML1 DTDs . If you don't have them installed locally you will get validation error messages when building in docs and the output will diverge. The simplest solution for you is probably to make sure you have XHTML1 DTDs installed in your local XML catalog, which on RHEL/Fedora is as simple as having the package xhtml1-dtds installed on your machine(s). The patch enclosed adds detection for the XHTML1 dtds at runtime on the build machine, and should solve the problem of diverging docs, replace the XML validity warnings by a more general message and add the requirement when building the RPM. I guess this should solve the issue Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list