Re: [libvirt] PATCH: Improve python exception messages
On Thu, Aug 21, 2008 at 12:00:46PM +0100, Daniel P. Berrange wrote: This patch was flawed - it missed out the error message info when passed in a domain/network/volume/pool instead of a connection object. Here is an updated patch which addresses that +1. I added the original '.. failed' messages. Before that there was no exception on failure at all :-) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Allow specific FDs to be kept open in virExec()
On Thu, Aug 21, 2008 at 02:58:30PM +0100, Daniel P. Berrange wrote: +#define VIR_EXEC_FDSET_SIZE() (sysconf(_SC_OPEN_MAX)/8) +#define VIR_EXEC_FDSET_CLEAR(set) memset((set), 0, VIR_EXEC_FDSET_SIZE()) +#define VIR_EXEC_FDSET_ON(set, fd) (set[(fd)/8] |= (1 ((fd)%8))) +#define VIR_EXEC_FDSET_OFF(set, fd) (set[(fd)/8] = ~(1 ((fd)%8))) +#define VIR_EXEC_FDSET_ISON(set, fd) (set[(fd)/8] (1 ((fd)%8))) +1 although I'm interested to know why you didn't just use ordinary FD_SETs for this. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] autobuild.sh: Fix minor shell-quoting bugs.
On Thu, Aug 21, 2008 at 05:57:37PM +0200, Jim Meyering wrote: I noticed some minor quoting problems in ovirt's autobuild.sh, and since part of that code came from here, ... +1 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] PATCH: Allow specific FDs to be kept open in virExec()
On Fri, Aug 22, 2008 at 09:22:06AM +0100, Richard W.M. Jones wrote: On Thu, Aug 21, 2008 at 02:58:30PM +0100, Daniel P. Berrange wrote: +#define VIR_EXEC_FDSET_SIZE() (sysconf(_SC_OPEN_MAX)/8) +#define VIR_EXEC_FDSET_CLEAR(set) memset((set), 0, VIR_EXEC_FDSET_SIZE()) +#define VIR_EXEC_FDSET_ON(set, fd) (set[(fd)/8] |= (1 ((fd)%8))) +#define VIR_EXEC_FDSET_OFF(set, fd) (set[(fd)/8] = ~(1 ((fd)%8))) +#define VIR_EXEC_FDSET_ISON(set, fd) (set[(fd)/8] (1 ((fd)%8))) +1 although I'm interested to know why you didn't just use ordinary FD_SETs for this. For some reason I have it in my head that fd_set was limited to 32 but looking at the Linux source its clearly 1024, which is the sme as my _SC_OPEN_MAX. The manpage says its FD_SETSIZE, but doesn't clarify whether FD_SETSIZE is guarenteed to be large enough to store any FD upto _SC_OPEN_MAX. Jim, any thoughts on this ? I'll happily switch to fd_set if we expect it'll be suitable 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 disk target collision when parsing domain xml
On Thu, Aug 21, 2008 at 11:20:13PM -0400, Cole Robinson wrote: The current domain xml parsing doesn't check if disks are specified with duplicate targets (hda etc.). The attached patch adds a check for this. +1. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 60 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add success output to virsh attach/detach commands
On Fri, Aug 22, 2008 at 09:26:13AM +0100, Richard W.M. Jones wrote: On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote: The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback. Hmmm ... The patch is fine, but I don't really like commands which are verbose on anything other than errors. Nor do I, but all our other commands print a confirmation message too, so for sake of consistency I think Cole's patch is reasonable. We do have a --quiet arg to make it STFU 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] Set correct device type when parsing input devices
On Thu, Aug 21, 2008 at 11:19:47PM -0400, Cole Robinson wrote: In virDomainDeviceDefParse, parsing an input device was actually setting it's type as DEVICE_DISK. The attached patch fixes this. 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] [PATCH] Check disk target collision when parsing domain xml
On Thu, Aug 21, 2008 at 11:20:13PM -0400, Cole Robinson wrote: The current domain xml parsing doesn't check if disks are specified with duplicate targets (hda etc.). The attached patch adds a check for this. Thanks, Cole commit 27df1653474738a2ce83c89e7bdb2c4f7327f9b6 Author: Cole Robinson [EMAIL PROTECTED] Date: Thu Aug 21 14:58:04 2008 -0400 Check for duplicate disk targets when parsing domain xml. diff --git a/src/domain_conf.c b/src/domain_conf.c index 6b23474..ed6cc8b 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -1955,6 +1955,12 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, } else { virDomainDiskDefPtr ptr = def-disks; while (ptr) { +if (ptr-next STREQ(disk-dst, ptr-next-dst)) { I'm confused as to why you're comparing for dups against ptr-next-dst instead of ptr-dst. It would seem to miss a duplication disk target check for the very first device.. +virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _(duplicate disk target '%s'), + disk-dst); +goto error; +} if (!ptr-next || virDomainDiskCompare(disk, ptr-next) 0) { disk-next = ptr-next; ptr-next = disk; 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] Fix cd eject segfault
On Thu, Aug 21, 2008 at 11:20:28PM -0400, Cole Robinson wrote: The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this. Good catch, +1 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
[libvirt] [PATCH] enable parallel builds
I'm not sure if this is the best way to do this, but it seems to work. Enable parallel compilation of the repository when running the autobuild script and/or via rpmbuild. --- autobuild.sh| 15 ++- libvirt.spec.in |2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 7ae5d1e..1a5413b 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -21,7 +21,20 @@ rm -rf coverage --with-lxc \ --with-xen-proxy -make +# from the gdc spec file +# if RPM_BUILD_NCPUS unset, set it +if [ -z $RPM_BUILD_NCPUS ] ; then + if [ -x /usr/bin/getconf ] ; then +RPM_BUILD_NCPUS=$(/usr/bin/getconf _NPROCESSORS_ONLN) +if [ $RPM_BUILD_NCPUS -eq 0 ]; then + RPM_BUILD_NCPUS=1 +fi + else +RPM_BUILD_NCPUS=1 + fi +fi + +make -j$RPM_BUILD_NCPUS make install set -o pipefail diff --git a/libvirt.spec.in b/libvirt.spec.in index d37c0e0..cd8e937 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -169,7 +169,7 @@ of recent versions of Linux (and other OSes). --with-init-script=redhat \ --with-qemud-pid-file=%{_localstatedir}/run/libvirt_qemud.pid \ --with-remote-file=%{_localstatedir}/run/libvirtd.pid -make +make %{?_smp_mflags} %install rm -fr %{buildroot} -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix cd eject segfault
On Thu, Aug 21, 2008 at 11:20:28PM -0400, Cole Robinson wrote: The cdrom eject code was trying to dereference the NULL source of an empty cdrom. Attached patch fixes this. @@ -2972,6 +2973,13 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, } VIR_FREE(safe_path); +newsrc = strdup(newdisk-src); +if (!newsrc) { +qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + %s, _(out of memory)); +return -1; +} Rather than dup'ing the string here and having to deal with OOM... + } else if (asprintf(cmd, eject cdrom) == -1) { qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED, %s, _(out of memory)); @@ -2982,11 +2990,17 @@ static int qemudDomainChangeCDROM(virDomainPtr dom, qemudReportError(dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED, %s, _(cannot change cdrom media)); VIR_FREE(cmd); +VIR_FREE(newsrc); return -1; } VIR_FREE(reply); VIR_FREE(cmd); -strcpy(olddisk-src, newdisk-src); + +VIR_FREE(olddisk-src); +if (newsrc) { +olddisk-src = newsrc; +newsrc = NULL; +} Just do VIR_FREE(olddisk-src); olddisk-src = newdisk-src; newdisk-src = NULL; 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] Fix disk device ordering
On Thu, Aug 21, 2008 at 11:21:15PM -0400, Cole Robinson wrote: When parsing the domain xml, disks are supposed to be rearranged in an alphabetic order based on bus type and target. The reordering code had a flaw though, in that it would always put the first disk it encountered at the head of the list, and had no way of putting new entries in front of it. The attached patch reworks the code to compare the new disk against the current disk entry (rather than the next entry like the existing code). Seems to pass all my tests. Urgh, the perils of writing everything in C. +1, this code looks correct to me. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 60 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix disk device ordering
On Thu, Aug 21, 2008 at 11:21:15PM -0400, Cole Robinson wrote: When parsing the domain xml, disks are supposed to be rearranged in an alphabetic order based on bus type and target. The reordering code had a flaw though, in that it would always put the first disk it encountered at the head of the list, and had no way of putting new entries in front of it. Hmm, yes good catch. The attached patch reworks the code to compare the new disk against the current disk entry (rather than the next entry like the existing code). Seems to pass all my tests. If the test suite passes that's good enough for me - we have a test case which explicitly does lots of disks with different buses names for this. 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] Update domain xml after usb hotplug
On Thu, Aug 21, 2008 at 11:21:33PM -0400, Cole Robinson wrote: The recently added usb hostdev and mass storage device hotplug code doesn't append the devices to the running guests xml if the hotplug succeeds. The attached patch fixes this. Yes, completely forgot about this when reviewing it before. 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] [PATCH] enable parallel builds
On Fri, Aug 22, 2008 at 06:36:23PM +1000, James Morris wrote: I'm not sure if this is the best way to do this, but it seems to work. Sure, gets my vote. 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] anyone implementing host device enumeration?
On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote: Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? No one that I'm aware of - so go for it. FYI, one complication has come up since my initial proposal. HAL is going away, to be replaced by DeviceKit :-( That said they will co-exist and HAL will be around for a while and they will both expose very similar properties for devices so it shouldn't too much trouble. I think we just need to be conservative when deciding how many of the HAL properties we need to add in libvirt. I'd go for a pretty minimal set at first. 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] storage pool discovery
On Thu, Aug 21, 2008 at 10:29:43AM -0400, David Lively wrote: Hi Folks - Here's my second pass at storage pool discovery. I've taken Daniel's suggestion and made it return a single XML doc containing source elements rather than an array of pool docs (and also incorporated suggestions from Daniel V and Jim M). Note that the storage source name patch is closely related (without it, the source docs returned for logical pools aren't correct). Excellant, I think this patch looks more or less good to commit now. +static char * +virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn, +const char *srcSpec, +unsigned int flags ATTRIBUTE_UNUSED) ... +const char *prog[] = { SHOWMOUNT, --no-headers, --exports, NULL, NULL }; +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; I'd prefer that to be sources - we avoid capitals in the XML element names everywhere else, and in the few cases of joining words use an underscore, but I think plural form is OK for this. +static char * +virStorageBackendLogicalDiscoverPools(virConnectPtr conn, + const char *srcSpec ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +const char *prog[] = { VGS, --noheadings, -o, vg_name, NULL }; +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; We should #define these two tags in storage_backend.h so each driver doesn't need to dup them 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] introducing source name (for logical storage pools)
On Thu, Aug 21, 2008 at 10:17:49AM -0400, David Lively wrote: Oops - that was against an old base. Sorry. Here's the new one. Also fixed a few other issues ... Ok, this gets my vote to commit. 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] Update domain xml after usb hotplug
On Thu, Aug 21, 2008 at 11:21:33PM -0400, Cole Robinson wrote: The recently added usb hostdev and mass storage device hotplug code doesn't append the devices to the running guests xml if the hotplug succeeds. The attached patch fixes this. IIRC there was some question about whether it should actually do this. I think it should -- a user would expect that when you attach a device to a domain, the device should appear in the XML (and be persistent too). So I ACK this patch. This code pattern: +/* Find spot in domain definition where we will put the disk */ +ptr = vm-def-disks; +prev = (vm-def-disks); +while (ptr) { appears at least twice. What we need is a Set abstract type. Implementing it as a red-black self-balancing tree would avoid any unexpected surprises when someone's guest has 100s of devices attached. Rich. Ob-OCaml-hack. The Set type in OCaml, implemented as an RB-tree, comes with a formal proof of correctness which runs to 5000 lines and took two man-months to complete. Along the way they found bugs in the original implementation. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Allow specific FDs to be kept open in virExec()
On Fri, Aug 22, 2008 at 10:51:54AM +0200, Jim Meyering wrote: Daniel P. Berrange [EMAIL PROTECTED] wrote: On Fri, Aug 22, 2008 at 09:22:06AM +0100, Richard W.M. Jones wrote: On Thu, Aug 21, 2008 at 02:58:30PM +0100, Daniel P. Berrange wrote: +#define VIR_EXEC_FDSET_SIZE() (sysconf(_SC_OPEN_MAX)/8) +#define VIR_EXEC_FDSET_CLEAR(set) memset((set), 0, VIR_EXEC_FDSET_SIZE()) +#define VIR_EXEC_FDSET_ON(set, fd) (set[(fd)/8] |= (1 ((fd)%8))) +#define VIR_EXEC_FDSET_OFF(set, fd) (set[(fd)/8] = ~(1 ((fd)%8))) +#define VIR_EXEC_FDSET_ISON(set, fd) (set[(fd)/8] (1 ((fd)%8))) +1 although I'm interested to know why you didn't just use ordinary FD_SETs for this. For some reason I have it in my head that fd_set was limited to 32 but looking at the Linux source its clearly 1024, which is the sme as my _SC_OPEN_MAX. The manpage says its FD_SETSIZE, but doesn't clarify whether FD_SETSIZE is guarenteed to be large enough to store any FD upto _SC_OPEN_MAX. Jim, any thoughts on this ? I'll happily switch to fd_set if we expect it'll be suitable AFAIK, FD_SET must work for any valid file descriptor. The fd_set bit-manipulation function/macros like FD_SET, FD_CLR are specified in http://www.opengroup.org/susv3xsh/select.html Ok, I'll redo this patch 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] configure.in: Prepend -lpthread to LIBS if required.
On Thu, Aug 21, 2008 at 08:23:21PM +0200, Jim Meyering wrote: Jun Koi [EMAIL PROTECTED] wrote: The latest cvs version has an error when compiling: make[3]: Entering directory `/home/jun/projects/libvirt-0821/src' /bin/bash ../libtool --tag=CC --mode=link gcc -g -O2 -Wall ... ./.libs/libvirt.so: undefined reference to `pthread_sigmask' collect2: ld returned 1 exit status make[3]: *** [virsh] Error 1 Thanks for the report. Next time, please tell us about your system, because I didn't 'see any such problem on a rawhide-based build. However, I did reproduce it on a Debian/unstable system. I think what's going on here is some linker fun with libtool / pkgconfig On Fedora it appears we end up automatically linked against pthread.so as a result of libxml2 (or some othe lib) being linked against it. I expect that Debian isn't doing this implicit linkage for us. In any case we should rely on Fedora behaviour, so this patch looks reasonable to me. 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] enable parallel builds
James Morris [EMAIL PROTECTED] wrote: I'm not sure if this is the best way to do this, but it seems to work. Enable parallel compilation of the repository when running the autobuild script and/or via rpmbuild. --- autobuild.sh| 15 ++- libvirt.spec.in |2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 7ae5d1e..1a5413b 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -21,7 +21,20 @@ rm -rf coverage --with-lxc \ --with-xen-proxy -make +# from the gdc spec file +# if RPM_BUILD_NCPUS unset, set it +if [ -z $RPM_BUILD_NCPUS ] ; then + if [ -x /usr/bin/getconf ] ; then +RPM_BUILD_NCPUS=$(/usr/bin/getconf _NPROCESSORS_ONLN) +if [ $RPM_BUILD_NCPUS -eq 0 ]; then + RPM_BUILD_NCPUS=1 +fi + else +RPM_BUILD_NCPUS=1 + fi +fi Good idea. I'll be happy to commit it with something like the following in place of the above: # If the MAKEFLAGS envvar does not yet include a -j option, # add -jN where N depends on the number of processors. case $MAKEFLAGS in *-j*) ;; *) n=$(getconf _NPROCESSORS_ONLN 2 /dev/null) test $n -gt 0 || n=1 n=`expr $n + 1` MAKEFLAGS=$MAKEFLAGS -j$n export MAKEFLAGS ;; esac Then you don't have to change the make invocation below, and it won't interfere if someone has already set MAKEFLAGS. Also, not using an absolute path to getconf lets that program work even also when it's installed in a different location. Finally, I prefer to use N_CPUS+1 as the -j option, here. (personally, I use 2*$N_CPUS+1, but that's probably too aggressive) Ok? +make -j$RPM_BUILD_NCPUS make install set -o pipefail Here's the patch I tested: diff --git a/autobuild.sh b/autobuild.sh index ce12692..cb6101f 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -21,6 +21,18 @@ rm -rf coverage --with-lxc \ --with-xen-proxy +# If the MAKEFLAGS envvar does not yet include a -j option, +# add -jN where N depends on the number of processors. +case $MAKEFLAGS in + *-j*) ;; + *) n=$(getconf _NPROCESSORS_ONLN 2 /dev/null) +test $n -gt 0 || n=1 +n=`expr $n + 1` +MAKEFLAGS=$MAKEFLAGS -j$n +export MAKEFLAGS +;; +esac + make make install diff --git a/libvirt.spec.in b/libvirt.spec.in index 4aff4a5..7c151ef 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -157,7 +157,7 @@ of recent versions of Linux (and other OSes). --with-init-script=redhat \ --with-qemud-pid-file=%{_localstatedir}/run/libvirt_qemud.pid \ --with-remote-file=%{_localstatedir}/run/libvirtd.pid -make +make %{?_smp_mflags} %install rm -fr %{buildroot} -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
RE: [libvirt] Using Xen config files
Xen and Xen-devel are 3.1.2. I'm using version 0.4.4 of libvirt. -matthew -Original Message- From: Richard W.M. Jones [mailto:[EMAIL PROTECTED] Sent: Friday, August 22, 2008 4:19 AM To: Matthew Donovan Cc: Daniel P. Berrange; libvir-list@redhat.com Subject: Re: [libvirt] Using Xen config files On Thu, Aug 21, 2008 at 02:16:55PM -0400, Matthew Donovan wrote: I'm still having problems with this. I've dug around and found errors in the qemu-dm log. When I try to start the VM with libvirt, the error I'm getting is: xs_read(/vm/414e73de-cf1e-487c-87e0-d4ebf7a23576/rtc/timeoffs et): read error I did a xenstore-ls on the /vm/414e73de-cf1e-487c-87e0-d4ebf7a23576 directory and sure enough, rtc/timeoffset is not there. When I start the VM with the Xen config file I used to generate the XML the rtc/timeoffset is there (timeoffset is set to 0). Which version of xen, xen-devel, etc? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add success output to virsh attach/detach commands
Daniel P. Berrange wrote: On Fri, Aug 22, 2008 at 09:26:13AM +0100, Richard W.M. Jones wrote: On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote: The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback. Hmmm ... The patch is fine, but I don't really like commands which are verbose on anything other than errors. Nor do I, but all our other commands print a confirmation message too, so for sake of consistency I think Cole's patch is reasonable. We do have a --quiet arg to make it STFU That was my reasoning as well. When commands like 'start' and 'define' report success but attach-device doesn't, I was left wondering if it even worked. - Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: REmove bogus call to virStateInit in openvz driver
On Tue, Aug 19, 2008 at 02:12:39PM +0100, Daniel P. Berrange wrote: For some unknown reason there is a call to virStateInitialize() in the openvz driver's open method. This is absolutely forbidden - this API is only intended for use by the daemon. This patch removes this bogus call and makes it call the openvz setup APIs directly. Okay, I think I understand the problem, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Improve python exception messages
On Tue, Aug 19, 2008 at 10:31:34AM +0100, Daniel P. Berrange wrote: Most of the libvirt python API bindings use code snippet like this when raising an exception: if ret is None:raise libvirtError('virConnectOpen() failed') THis sets the message associated with the exception to virConnectOpen() failed This contains essentially zero useful information - you can see that it was virConnectOpen which failed from the stack trace. Now the libvirt error object has a real message, such as authentication failed Or unable to connect to '/var/run/libvirt/libvirt-sock': Connection refused This patch makes sure we extract this real error message and use it to set the message associated with the exception object. This is one step in getting better error reporting for virt-install/virt-manager, which is particularly needed for remote connections Good idea, +1, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Ensure errors are guarenteed reported in virConnectOpen
On Tue, Aug 19, 2008 at 11:35:18AM +0100, Daniel P. Berrange wrote: The virConnectOpen method is unfortuantely rather special. While there is a virConnect object available, the current rule is that drivers must report errors against the global error object, because upon failure no virConnect object will be returned to the caller. Unforatunately, despite frequent cleanups of code getting this wrong, I've just audited the remote driver and found another 20 or so places where its wrong. This is clearly not a sustainable approach to error reporting. The guarenteed correct solution is actually rather simple - Always report errors against the virConnect object, even in the driver open method - In the cleanup patch of do_open() in libvirt.c, if no global error is set, copy the error from the virConnect object's virError, into the global virError. With this in place we can change all the drivers back to always report against the error object, and thus cleanup some disgusting code like __virRaiseError (in_open ? NULL : conn, ... To just __virRaiseError (conn, ... Okay, make sense, it's better to be more flexible in one location and simplify code everywhere else. +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] static ip address
On Fri, Aug 15, 2008 at 10:43:15AM +0100, Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 05:24:01PM -0400, Daniel Veillard wrote: New patch with just a pair of minor fixes which seems to work just fine for me. The syntax is nearly the old one, just use host instead of statichost: ip address=192.168.122.1 netmask=255.255.255.0 dhcp range start=192.168.122.4 end=192.168.122.254 / host mac=00:16:3E:XX:XX:XX host=XXX Seems a little odd to have 'host' as the element name and attribute name at once. Perhaps the atribute should just be name='' ? Agreed, I noticed that but it's not a serious problem, still using name feels cleaner I will change that before commiting ! @@ -1103,6 +1103,8 @@ 2 + /* --listen-address 10.0.0.1 */ 1 + /* --dhcp-leasefile=path */ (2 * network-def-nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ +/* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ +(2 * network-def-nhosts) + Using the --dhcp-host option means we can't easily update on the fly, but this patch doesn't support on the fly updates anyway, so not a huge deal. In the future we can switch to --dhcp-host-file instead. ACK to including this. okay, will push later today, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Fri, Aug 22, 2008 at 09:44:47AM +0100, Daniel P. Berrange wrote: On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote: Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? No one that I'm aware of - so go for it. FYI, one complication has come up since my initial proposal. HAL is going away, to be replaced by DeviceKit :-( That said they will co-exist and HAL will be around for a while and they will both expose very similar properties for devices so it shouldn't too much trouble. I think we just need to be conservative when deciding how many of the HAL properties we need to add in libvirt. I'd go for a pretty minimal set at first. And HAL based detection might work on more platforms at least initially like OpenSolaris, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; I'd prefer that to be sources - we avoid capitals in the XML element names everywhere else, and in the few cases of joining words use an underscore, but I think plural form is OK for this. I prefer sources as well, so I'll change this. And I'll put those defines in storage_backend.h as well. As long as we're on the subject of naming (and before it's too late), it's been bothering me that we keep calling this storage pool discovery. To me, storage source discovery seems more accurate (because they're not pools until we define libvirt pools based on the sources). So I'd prefer renaming the various *Discover[Storage]Pools* functions (and support structs) introduced in this patch to *Discover[Storage]Sources*. I was just sticking with the originally-proposed names to avoid confusion. What do you all think? Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
Here's the patch with those issues addressed (also merged with latest upstream - avoids internal.h merge conflict) ... Dave On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: On Thu, Aug 21, 2008 at 10:29:43AM -0400, David Lively wrote: Hi Folks - Here's my second pass at storage pool discovery. I've taken Daniel's suggestion and made it return a single XML doc containing source elements rather than an array of pool docs (and also incorporated suggestions from Daniel V and Jim M). Note that the storage source name patch is closely related (without it, the source docs returned for logical pools aren't correct). Excellant, I think this patch looks more or less good to commit now. +static char * +virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn, +const char *srcSpec, +unsigned int flags ATTRIBUTE_UNUSED) ... +const char *prog[] = { SHOWMOUNT, --no-headers, --exports, NULL, NULL }; +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; I'd prefer that to be sources - we avoid capitals in the XML element names everywhere else, and in the few cases of joining words use an underscore, but I think plural form is OK for this. +static char * +virStorageBackendLogicalDiscoverPools(virConnectPtr conn, + const char *srcSpec ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +const char *prog[] = { VGS, --noheadings, -o, vg_name, NULL }; +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; We should #define these two tags in storage_backend.h so each driver doesn't need to dup them Daniel diff --git a/configure.in b/configure.in index 9479f1c..430a097 100644 --- a/configure.in +++ b/configure.in @@ -671,6 +671,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi AC_PATH_PROG([QEMU_IMG], [qemu-img], [], [$PATH:/sbin:/usr/sbin:/bin:/usr/bin]) if test -n $QEMU_IMG ; then diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 9c3e1c2..5308fa9 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn, int maxnames); /* + * Query a host for storage pools of a particular type + */ +char * virConnectDiscoverStoragePools(virConnectPtr conn, + const char *type, + const char *srcSpec, + unsigned int flags); + +/* * Lookup pool by name or uuid */ virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f077a26..3d06d76 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn, int maxnames); /* + * Query a host for storage pools of a particular type + */ +char * virConnectDiscoverStoragePools(virConnectPtr conn, + const char *type, + const char *srcSpec, + unsigned int flags); + +/* * Lookup pool by name or uuid */ virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn, diff --git a/qemud/remote.c b/qemud/remote.c index b5a6ec9..43b3a56 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -2958,6 +2958,27 @@ remoteDispatchListStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchDiscoverStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED, +struct qemud_client *client, +remote_message_header *req, +remote_discover_storage_pools_args *args, +remote_discover_storage_pools_ret *ret) +{ +CHECK_CONN(client); + +ret-xml = +virConnectDiscoverStoragePools (client-conn, +args-type, +args-srcSpec ? *args-srcSpec : NULL, +
Re: [libvirt] [PATCH] storage pool discovery
On Fri, Aug 22, 2008 at 10:58:54AM -0400, David Lively wrote: On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; I'd prefer that to be sources - we avoid capitals in the XML element names everywhere else, and in the few cases of joining words use an underscore, but I think plural form is OK for this. I prefer sources as well, so I'll change this. And I'll put those defines in storage_backend.h as well. As long as we're on the subject of naming (and before it's too late), it's been bothering me that we keep calling this storage pool discovery. To me, storage source discovery seems more accurate (because they're not pools until we define libvirt pools based on the sources). So I'd prefer renaming the various *Discover[Storage]Pools* functions (and support structs) introduced in this patch to *Discover[Storage]Sources*. I was just sticking with the originally-proposed names to avoid confusion. What do you all think? That sounds like a reasonable idea to me. 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] Delete veth devices during lxcVMCleanup
DB All my patches for LXC with exception of the pivot_root stuff are DB now commited, so this is good to commit. Done. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: [EMAIL PROTECTED] pgpg7jtngAkLp.pgp Description: PGP signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
David Lively [EMAIL PROTECTED] wrote: Here's the patch with those issues addressed (also merged with latest upstream - avoids internal.h merge conflict) ... Hi David, Looks good. A couple of minor suggestions and questions. ... diff --git a/configure.in b/configure.in index 9479f1c..430a097 100644 --- a/configure.in +++ b/configure.in @@ -671,6 +671,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi Do we want to require the showmount package via the spec file? ... diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c ... +static int +virStorageBackendFileSystemNetDiscoverPoolsFunc(virConnectPtr conn ATTRIBUTE_UNUSED, +virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +char **const groups, +void *data) +{ +virNetfsDiscoverState *state = data; +virStringList *newItem; +const char *name, *path; path can be const, too. + +path = groups[0]; + +name = strrchr(path, '/'); +if (name == NULL) { +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _(invalid netfs path (no slash): %s), path); +return -1; +} +name += 1; It'd be good to diagnose a path that ends in a slash (*name == '\0'), rather than emitting XML with an empty name, below. +/* Append new XML desc to list */ + +if (VIR_ALLOC(newItem) != 0) { +virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(new xml desc)); +return -1; +} + +if (asprintf(newItem-val, + sourcehost name='%s'/dir path='%s'//source\n, + state-host, path) = 0) { +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(asprintf failed)); +VIR_FREE(newItem); +return -1; +} + +newItem-len = strlen(newItem-val); +newItem-next = state-list; +state-list = newItem; + +return 0; +} + +static char * +virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn, +const char *srcSpec, +unsigned int flags ATTRIBUTE_UNUSED) +{ +/* + * # showmount --no-headers -e HOSTNAME + * /tmp * + * /A dir demo1.foo.bar,demo2.foo.bar + * + * Extract directory name (including possible interior spaces ...). + */ + +const char *regexes[] = { +^(/.*\\S) +\\S+$ +}; +int vars[] = { +1 +}; +xmlDocPtr doc = NULL; +xmlXPathContextPtr xpath_ctxt = NULL; +virNetfsDiscoverState state = { .host = NULL, .list = NULL }; +const char *prog[] = { SHOWMOUNT, --no-headers, --exports, NULL, NULL }; +int exitstatus, len; Please use size_t as the type for string-length variables like len. +virStringList *p; +char *retval = NULL; + +doc = xmlReadDoc((const xmlChar *)srcSpec, srcSpec.xml, NULL, + XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); +if (doc == NULL) { +virStorageReportError(conn, VIR_ERR_XML_ERROR, %s, _(bad source spec)); +goto cleanup; +} + +xpath_ctxt = xmlXPathNewContext(doc); +if (xpath_ctxt == NULL) { +virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(xpath_ctxt)); +goto cleanup; +} + +state.host = virXPathString(conn, string(/source/host/@name), xpath_ctxt); +if (!state.host || !state.host[0]) { +virStorageReportError(conn, VIR_ERR_XML_ERROR, %s, + _(missing host in source spec)); +goto cleanup; +} +prog[3] = state.host; + +if (virStorageBackendRunProgRegex(conn, NULL, prog, 1, regexes, vars, + virStorageBackendFileSystemNetDiscoverPoolsFunc, + state, exitstatus) 0) +goto cleanup; + +/* Turn list of strings into final document */ +len = strlen(SOURCES_START_TAG) + strlen(SOURCES_END_TAG); +for (p = state.list; p; p = p-next) +len += p-len; +if (VIR_ALLOC_N(retval, len+1) 0) { +virStorageReportError(conn, VIR_ERR_NO_MEMORY, _(retval (%d bytes)), len); +goto cleanup; +} +strcpy(retval, SOURCES_START_TAG); +len = strlen(SOURCES_START_TAG); +for (p = state.list; p; p = p-next) { +strcpy(retval + len, p-val); +len += p-len; +} +strcpy(retval + len, SOURCES_END_TAG); + + cleanup: +
Re: [libvirt] [PATCH] kvm: maxVCPU runtime detection
Daniel P. Berrange [EMAIL PROTECTED] wrote: On Fri, Aug 22, 2008 at 03:16:42PM +0200, Guido G?nther wrote: Hi, with recent linux kernels we can detect the maximum number of virtual cpus at runtime via an ioctl. Possible patch attached. It does this on every call to qemudGetMaxVCPUs. Would you prefer something that does this only once in qemudStartup()? Cheers, -- Guido +dnl +dnl check for kvm headers +dnl +AC_CHECK_HEADERS([linux/kvm.h]) Hmm, I wonder if that's commonly installed by Fedora/Debian RPMs for KVM I've just checked up to date rawhide and debian unstable systems. Both had this: $ grep KVM_CHECK_EXTENSION /usr/include/linux/kvm.h #define KVM_CHECK_EXTENSION _IO(KVMIO, 0x03) But rawhide lacks KVM_CAP_NR_VCPUS, while Debian has it. $ grep -r KVM_CAP_NR_VCPUS /usr/include/linux/kvm.h #define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ It might be neccessary to just #define the IOCTL constant in our own header files if its not available. Good point. Otherwise, this patch looks fine. @@ -72,6 +77,9 @@ static int qemudShutdown(void); #endif +/* device for kvm ioctls */ +#define KVM_DEVICE /dev/kvm + /* qemudDebug statements should be changed to use this macro instead. */ #define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) #define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg) @@ -1768,6 +1776,29 @@ static const char *qemudGetType(virConnectPtr conn ATTRIBUTE_UNUSED) { return QEMU; } + +static int kvmGetMaxVCPUs(void) { +int maxvcpus = 1; + +#if defined(KVM_CAP_NR_VCPUS) +int r, fd; + +fd = open(KVM_DEVICE, O_RDONLY); +if (fd 0) { +qemudLog(QEMUD_WARN, _(Unable to open KVM_DEVICE : %s\n), strerror(errno)); +return maxvcpus; +} + +r = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS); +if (r 0) +maxvcpus = r; + +close(fd); +#endif +return maxvcpus; +} This is all very cool. IIRC Cole has some code which could guess a reasonable maxvcpus from KVM version number which we could potentially fallback to Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Allow specific FDs to be kept open in virExec()
Daniel P. Berrange [EMAIL PROTECTED] wrote: With my recent patches to virExec(), all FDs except stdin/out/err are closed before the child is exec'd to prevent accidental leaks. Of course I forgot the one key place where we need to propagate FDs... The TAP devices passed to QEMU. This patch adds a 'keepfd' parameter to virExec() which allows the caller to specify a bitset of file descriptors to keep open, and updates the callers to use this as required. The QEMU driver specifies any TAP fds and the LXC driver specifies its PTY this way removing a previous hack. This all looks fine, assuming you're adjusting it to use the standard fd_set type. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] kvm: maxVCPU runtime detection
On Fri, Aug 22, 2008 at 07:26:12PM +0200, Jim Meyering wrote: [..snip..] I've just checked up to date rawhide and debian unstable systems. Both had this: $ grep KVM_CHECK_EXTENSION /usr/include/linux/kvm.h #define KVM_CHECK_EXTENSION _IO(KVMIO, 0x03) But rawhide lacks KVM_CAP_NR_VCPUS, while Debian has it. $ grep -r KVM_CAP_NR_VCPUS /usr/include/linux/kvm.h #define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ Debian has it only in unstable not in testing. That's why I added the header check as well as the #if defined(KVM_CAP_NR_VCPUS) - the code stays as is for old header files and uses the definition (and therefore the ioctl) if available. Rawhide will get the defintion with the next libc header update I guess. -- Guido -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
Hi Jim - Thanks for your comments. I really appreciate the detailed look. I'll implement your suggestions (including the refactoring of the code to turn a virStringList into a single XML string), though I have a question about one of them: On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: diff --git a/configure.in b/configure.in index 9479f1c..430a097 100644 --- a/configure.in +++ b/configure.in @@ -671,6 +671,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi Do we want to require the showmount package via the spec file? I was a little worried about this too. It's probably better to handle this more like iscsiadmin/--with-storage-iscsi: defaults to enabling iscsi backend if and only if iscsiadmin is present. Fails with --with-storage-iscsi=yes explicitly specified and iscsiadmin not found. But I'm not sure we want to disable the storage_fs backend just because we can't find showmount. So maybe there should be another config option --with-storage-netfs-discovery to cover the only code that actually uses showmount? Either of these sounds reasonable to me. Do you have a preference? Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: +const char *name, *path; path can be const, too. It is, at least according to my gcc (4.3.0-8, x86_64, Fedora9). Compile the following and without FORCE_WARNING to check: void check() { #ifdef FORCE_WARNING const char *foo; char *bar; #else const char *foo, *bar; #endif foo = I'm a const char *; bar = foo; // warns iff bar not const char * } Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
David Lively [EMAIL PROTECTED] wrote: On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: +const char *name, *path; path can be const, too. It is, at least according to my gcc (4.3.0-8, x86_64, Fedora9). Compile the following and without FORCE_WARNING to check: void check() { #ifdef FORCE_WARNING const char *foo; char *bar; #else const char *foo, *bar; #endif Oops. Now you know why I prefer to declare them on separate lines: one less rule to remember that way ;-) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
David Lively [EMAIL PROTECTED] wrote: ... AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi Do we want to require the showmount package via the spec file? I was a little worried about this too. It's probably better to handle this more like iscsiadmin/--with-storage-iscsi: defaults to enabling iscsi backend if and only if iscsiadmin is present. Fails with --with-storage-iscsi=yes explicitly specified and iscsiadmin not found. Adding Requires: showmount and BuildRequires lines to libvirt.spec.in would be similar to the approach of requiring the iscsi-initiator-utils package (which contains iscsiadm): $ grep isc *.spec.in Requires: iscsi-initiator-utils BuildRequires: iscsi-initiator-utils -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
On Fri, Aug 22, 2008 at 11:06:38PM +0200, Jim Meyering wrote: David Lively [EMAIL PROTECTED] wrote: ... AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi Do we want to require the showmount package via the spec file? I was a little worried about this too. It's probably better to handle this more like iscsiadmin/--with-storage-iscsi: defaults to enabling iscsi backend if and only if iscsiadmin is present. Fails with --with-storage-iscsi=yes explicitly specified and iscsiadmin not found. Adding Requires: showmount and BuildRequires lines to libvirt.spec.in would be similar to the approach of requiring the iscsi-initiator-utils package (which contains iscsiadm): Yes, basically the spec file should encode 'best practice' and as such it is reasonable to add a 'Requires: showmount'. 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