[libvirt] [PATCH] xen: Prevent updating device when attaching a device
When attaching a device that already exists, xend driver updates the device with device_configure, it causes problems (e.g. for disk device, it only can be used to update device like CDROM), and actually we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device. * src/xen/xend_internal.c --- src/xen/xend_internal.c | 36 +--- 1 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 6ce0c3f..c71aeb3 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3956,6 +3956,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; +char *target; if ((domain == NULL) || (domain-conn == NULL) || (domain-name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4020,6 +4021,8 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def-os.type, hvm) ? 1 : 0, priv-xendConfigVersion, 1) 0) goto cleanup; + +target = dev-data.disk-dst; break; case VIR_DOMAIN_DEVICE_NET: @@ -4029,6 +4032,12 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def-os.type, hvm) ? 1 : 0, priv-xendConfigVersion, 1) 0) goto cleanup; + +if (virParseMacAddr(target, dev-data.net-mac) 0) { +virXendError(VIR_ERR_INTERNAL_ERROR, + _(malformed mac address '%s'), target); +goto cleanup; +} break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -4037,13 +4046,18 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, if (xenDaemonFormatSxprOnePCI(dev-data.hostdev, buf, 0) 0) goto cleanup; -} else { -virXendError(VIR_ERR_NO_SUPPORT, %s, - _(unsupported device type)); -goto cleanup; -} -break; +virDomainDevicePCIAddress PCIAddr; + +PCIAddr = dev-data.hostdev-source.subsys.u.pci; +virAsprintf(target, PCI device: %.4x:%.2x:%.2x, PCIAddr.domain, + PCIAddr.bus, PCIAddr.slot); + +if (target == NULL) { +virReportOOMError(); +goto cleanup; +} +} default: virXendError(VIR_ERR_NO_SUPPORT, %s, _(unsupported device type)); @@ -4056,17 +4070,17 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, /* device doesn't exist, define it */ ret = xend_op(domain-conn, domain-name, op, device_create, config, sexpr, NULL); -} -else { -/* device exists, attempt to modify it */ -ret = xend_op(domain-conn, domain-name, op, device_configure, - config, sexpr, dev, ref, NULL); +} else { +virXendError(VIR_ERR_OPERATION_INVALID, + _(target '%s' already exists), target); } cleanup: VIR_FREE(sexpr); virDomainDefFree(def); virDomainDeviceDefFree(dev); +if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) +VIR_FREE(target); return ret; } -- 1.7.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 7/7] build: make building on cygwin easier
On 21/12/2010, at 6:52 AM, Eric Blake wrote: ACK to your additions, and coupled with your ACK of the rest of my patch, this passed testing on Fedora 14, RHEL 5, a Linux-hosted cross-build to mingw, and a native cygwin build, so I've gone ahead and pushed it. Thanks guys. This also fixed the dlopen() problem in ./configure on OSX too, mentioned yesterday, as Eric suggested it might. ./configure working fine now. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: skip vmware driver when building for RHEL
* libvirt.spec.in: Provide vmware conditionals. --- Would you mind preparing a followup patch that modifies libvirt.spec.in to make it configurable when building an rpm whether vmware support is built in? See commit e3e31303d54e for an example. I went ahead and did this, since it was breaking the 'make rpm' phase of ./autobuild.sh. However, I'd like a review before I push. libvirt.spec.in |9 - 1 files changed, 8 insertions(+), 1 deletions(-) Thanks for doing that. ACK. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/1] build: skip drivers during mingw portion of autobuild
* autobuild.sh: Alter mingw configuration setup. --- And a followup to allow autobuild.sh completion when a mingw cross-toolchain is present. autobuild.sh |2 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 91e2ab2..4cb9fde 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -83,6 +83,8 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then --without-openvz \ --without-one \ --without-phyp \ +--without-xenapi \ +--without-vmware \ +--without-esx \ --without-netcf \ --without-audit \ --without-dtrace \ ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Support for Parallels products?
Hi all, Has anyone looked at adding support for Parallels virtualisation products, or have any idea if it's even possible? Google isn't showing much in the way of search results, so not sure it's been looked at before. These are the server oriented products: http://www.parallels.com/au/products/server/ They have a hosted hypervisor (like VMware GSX/Server), and a bare metal hypervisor. They also have desktop oriented products: http://www.parallels.com/au/computing/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Anyone interested in forming a libvirt docs team?
Hi all, There's a lot of documentation for libvirt that needs improving, so wondering if anyone would be interested in forming a libvirt docs team to help make that happen? While some of the documentation needs people familiar with specific pieces of libvirt, there's also a lot that doesn't, and just needs people with some free time and willingness to help. Anyone up for it? :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Anyone interested in forming a libvirt docs team?
On 12/21/2010 11:53 AM, Justin Clift wrote: Hi all, There's a lot of documentation for libvirt that needs improving, so wondering if anyone would be interested in forming a libvirt docs team to help make that happen? While some of the documentation needs people familiar with specific pieces of libvirt, there's also a lot that doesn't, and just needs people with some free time and willingness to help. Anyone up for it? :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Hello Justin, could you be more specific, please? How to put it. Your e-mail is too ambiguous for me :) Regards, Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] Anyone interested in forming a libvirt docs team?
Hi Justin, I can offer some free time :) Anthony Davis. On 21 Dec 2010, at 10:53, Justin Clift jcl...@redhat.com wrote: Hi all, There's a lot of documentation for libvirt that needs improving, so wondering if anyone would be interested in forming a libvirt docs team to help make that happen? While some of the documentation needs people familiar with specific pieces of libvirt, there's also a lot that doesn't, and just needs people with some free time and willingness to help. Anyone up for it? :) Regards and best wishes, Justin Clift ___ libvirt-users mailing list libvirt-us...@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-users -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] Skip file-based security checks for network disks
On 12/20/2010 07:30 PM, Josh Durgin wrote: Network disks are accessed by qemu directly, and have no associated file on the host, so checking for file ownership etc. is unnecessary. Signed-off-by: Josh Durgin jo...@hq.newdream.net --- src/conf/domain_conf.c |2 +- src/qemu/qemu_security_dac.c |2 +- src/security/security_apparmor.c |2 +- src/security/security_selinux.c |2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d516fbe..c857a89 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8353,7 +8353,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, size_t depth = 0; char *nextpath = NULL; -if (!disk-src) +if (!disk-src || disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) ACK and pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2] storage: Ignore dangling symbol link for filesystem pool
On 12/20/2010 11:47 PM, Osier Yang wrote: a more efficient solution would be to check if errno is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents. I guess you mean stat, lstat will not work here, as it doesn't follow the *symbolic* link. what we need to do is to determine if the symbolic link is dangling, so use stat to update the patch, v3 send, thanks again. No, I really meant lstat(). If stat() would have failed because of a dangling symlink, then open() will fail for the same reasons. Therefore, check errno before lstat, and use the successful lstat as proof that a symlink is in place but that stat()ing the symlink would fail because it is dangling. if (open(f) 0) { if ((errno == ENOENT || errno == ELOOP) lstat(f, buf) == 0) { /* Dangling symlink, since lstat() passed but open failed. */ log message about ignored file return -2; } either an unrelated errno, like EACCES, or we got ENOENT because the file was deleted after readdir but before open/lstat error message about unaccessible file return -1; } -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [v3] storage: Ignore dangling symbolic link for filesystem pool
On 12/20/2010 11:45 PM, Osier Yang wrote: If there is a dangling symbolic link in filesystem pool, the pool will fail to start or refresh, this patch is to fix it by ignoring it with a warning log. --- src/storage/storage_backend.c| 10 +- src/storage/storage_backend_fs.c |2 +- 2 files changed, 10 insertions(+), 2 deletions(-) @@ -986,6 +987,13 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) 0) { +if (stat(path, sb) 0 +(errno == ENOENT || errno == ELOOP)) { +VIR_WARN(_(cannot open volume '%s' :%s), path, + strerror(errno)); +return -2; +} Given my comments here: https://www.redhat.com/archives/libvir-list/2010-December/msg00826.html I'm squashing this in, then pushing. Thanks for tackling this one. diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c index efdd258..66775e9 100644 --- i/src/storage/storage_backend.c +++ w/src/storage/storage_backend.c @@ -987,8 +987,8 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) 0) { -if (stat(path, sb) 0 -(errno == ENOENT || errno == ELOOP)) { +if ((errno == ENOENT || errno == ELOOP) +lstat(path, sb) == 0) { VIR_WARN(_(cannot open volume '%s' :%s), path, strerror(errno)); return -2; -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [v3] storage: Ignore dangling symbolic link for filesystem pool
On 12/20/2010 11:45 PM, Osier Yang wrote: If there is a dangling symbolic link in filesystem pool, the pool will fail to start or refresh, this patch is to fix it by ignoring it with a warning log. if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) 0) { +if (stat(path, sb) 0 +(errno == ENOENT || errno == ELOOP)) { +VIR_WARN(_(cannot open volume '%s' :%s), path, Oh, and 'make syntax-check' didn't like _() marking inside VIR_WARN. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] Anyone interested in forming a libvirt docs team?
On 21/12/2010, at 10:15 PM, Anthony Davis wrote: Hi Justin, I can offer some free time :) Awesome Anthony, very welcome. :) There's a pretty broad spectrum of things we need to get improved. :) From stuff that doesn't take any real virtualisation knowledge: + The new Virsh Command Reference. Most pages just list the command name and the version of libvirt it came in, without even showing the arguments the command can be given. Like this: http://libvirt.org/sources/virshcmdref/html/sect-list.html It just needs someone(s) who can run the virsh help command for a command, and then write down the options the command has. We're using a really simple text format for this, so it's pretty easy. :) Stuff that requires knowledge of an operating system, but not necessarily deep Virtualisation knowledge: + The existing docs on the libvirt.org website have been (mostly) written by people using path names that apply to Fedora and RHEL. It would be good to identify these, and update these to include the correct paths for other Linux distributions too, such as Ubuntu, OpenSUSE, Gentoo, and so on. Probably the first thing that needs to be done for this is just someone to scan through the pages on the website making a list of which ones have paths to be looked at. We can then figure out from there what it'll take, who needs to do what, etc. :) + Screenshots. Lots of the virt-manager pieces and processes could benefit from having good screenshots taken to show the process. Note, I haven't really thought this bit through to a deep level, so we probably need to discuss, make a list, and so on. :) Stuff that could really use a person that *does* have solid knowledge about some aspect of things, or a strong desire to learn it (then write it up). The two areas that jump out the most as needing attention here are: + Virtualisation storage concepts. There are some initial bits around What is a storage pool? What is a storage volume? How do we use them? in places, but nothing really nicely put together, targeted to general SysAdmin users. Some of this info does exist in the Fedora or RHEL virtualisation guides, but they're distro specific and this info should be available on the libvirt.org or virt-tools.org websites themselves. + Networking and virtualisation. All kinds of stuff here, from explaining how each of the virtual networking types work, through to explaining what the VirtIO network device is, and how to install the drivers for it in Windows. All of which is directly of use to heaps of people, and hasn't yet been written. Heh. ;) So, I guess the first and best question is which of the above sounds like you? :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/1] build: skip drivers during mingw portion of autobuild
On 12/21/2010 03:11 AM, Jiri Denemark wrote: * autobuild.sh: Alter mingw configuration setup. --- And a followup to allow autobuild.sh completion when a mingw cross-toolchain is present. autobuild.sh |2 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 91e2ab2..4cb9fde 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -83,6 +83,8 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then --without-openvz \ --without-one \ --without-phyp \ +--without-xenapi \ +--without-vmware \ +--without-esx \ --without-netcf \ --without-audit \ --without-dtrace \ ACK Hmm, ./autobuild.sh is still failing for me, because it still tries to make the mingw rpm, which did not have these changes. The failure stems from the fact that configure tries to build vmware but not esx if curl/curl.h is missing, but vmware indirectly requires both esx and curl/curl.h at the moment: CC libvirt_driver_vmware_la-vmware_driver.lo In file included from vmware/../esx/esx_vmx.h:31:0, from vmware/vmware_driver.c:28: vmware/../esx/esx_vi.h:28:24: fatal error: curl/curl.h: No such file or directory compilation terminated. I don't think I need to disable esx (and I certainly don't want to disable it from the mingw specfile), so I'll try again with a v2, perhaps waiting until after Matthias' promised patch to clean up the current esx/vmware problems. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Anyone interested in forming a libvirt docs team?
On 21/12/2010, at 10:32 PM, Zdenek Styblik wrote: Hello Justin, could you be more specific, please? How to put it. Your e-mail is too ambiguous for me :) Heh, no worries. I often talk in general terms first. :) The email response to Anthony, sent a few minutes ago, has more detail. I know there's a lot of demand for better documentation in libvirt, and other Open Source projects around have gotten together teams/groups of people who's main mission is to improving their documentation/manuals/learning materials (and so on). So, I reckon we see about doing the same thing for libvirt. Looks like there are people willing to pitch in, so I guess it's now a matter of figuring out what needs to be done, who's interested in doing what, and so on. If we need a libvirt-docs mailing list too, that can be done without too much effort, but not sure it's needed just yet. (open to suggestions :)) Hmmm, that email reply to Anthony should probably be copied to the wiki and we can expand on it from there. :) Is this the kind of detail you're after Zdenek? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: skip vmware driver when building for RHEL
On 12/21/2010 02:59 AM, Jiri Denemark wrote: * libvirt.spec.in: Provide vmware conditionals. --- Would you mind preparing a followup patch that modifies libvirt.spec.in to make it configurable when building an rpm whether vmware support is built in? See commit e3e31303d54e for an example. I went ahead and did this, since it was breaking the 'make rpm' phase of ./autobuild.sh. However, I'd like a review before I push. libvirt.spec.in |9 - 1 files changed, 8 insertions(+), 1 deletions(-) Thanks for doing that. ACK. Pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] Anyone interested in forming a libvirt docs team?
On 22/12/2010, at 3:23 AM, Scott Baker wrote: On 12/21/2010 03:15 AM, Anthony Davis wrote: Hi Justin, I can offer some free time:) I'd be willing to contribute to a wiki if we had the docs in a wiki. Yeah I know what you mean. Wiki's are *so* much easier to work with and revise, compared to having to code in html then submit diffs as patches. At the moment, the main libvirt docs are written in html and included with each release tarball. It makes sure everyone who downloads the tarball has a copy, but it doesn't make them easy to edit. :/ Some of the things that need documenting though, are definitely able to be written up into a wiki first to get them into shape. We could then move the things that are finished in the wiki to the main tarball (after converting to html). I personally do similar to this anyway for lots of things, as I find getting info onto a wiki first allows me to refine, refine, refine, until I'm happy with it. Then it can go anywhere. This is the first tentative Docs Team To Do list page (on the wiki even), just whipped up using the contents of a reply to Anthony: http://wiki.libvirt.org/page/DocsToDo Stuff that I reckon would be suitable for writing on the wiki generally fall into the second and third categories there, of things that need either some OS level knowledge (for investigating non Fedora-RHEL paths), or for writing up concepts around Storage, Networking, or similar. You up for it? :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Move VMX handling code out of the driver directory
On 12/21/2010 09:03 AM, Matthias Bolte wrote: Now the VMware driver doesn't depend on the ESX driver anymore. Add a WITH_VMX option that depends on WITH_ESX and WITH_VMWARE. Also add a libvirt_vmx.syms file. Move some escaping functions from esx_util.c to vmx.c. Adapt the test suite, ESX and VMware driver to the new code layout. --- a/src/esx/esx_driver.c @@ -88,7 +88,7 @@ struct _esxVMX_Data { * Firstly this functions checks if the given file name contains a separator. * If it doesn't then the referenced file is in the same directory as the .vmx * file. The datastore name and directory of the .vmx file are passed to this - * function via the opaque paramater by the caller of esxVMX_ParseConfig. + * function via the opaque paramater by the caller of virVMXParseConfig. As long as you're touching this line, s/paramater/parameter/ +++ b/src/libvirt_vmx.syms @@ -0,0 +1,23 @@ +# +# These symbols are dependent upon --with-esx via WITH_ESX or --with-vmware via WITH_VMWARE. +# + +# vmx.h +virVMXConvertToUTF8; +virVMXEscapeHex; +virVMXUnescapeHex; +virVMXParseConfig; Do you want to sort this list? diff --git a/src/esx/esx_vmx.c b/src/vmx/vmx.c similarity index 79% rename from src/esx/esx_vmx.c rename to src/vmx/vmx.c index 5cbb835..6e3e9af 100644 'git diff' is so awesome with renames :) @@ -454,19 +456,23 @@ def-parallels[0]... */ -#define VIR_FROM_THIS VIR_FROM_ESX +#define VIR_FROM_THIS VIR_FROM_NONE Bit of a shame that we lose information here, but not the end of the world. ACK with the spelling and sorting nits addressed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Move VMX handling code out of the driver directory
On 22/12/2010, at 4:40 AM, Eric Blake wrote: snip 'git diff' is so awesome with renames :) Hmmm recently had a problem with it where it *insisted* that one of the 100+ files in a git repo I'd been messing with was a rename of a different file... even when it wasn't. (significantly different file contents) Didn't find an option to force it to recognise the file *wasn't* a rename, so from memory just had to leave it with it's incorrect rename entry in history. :( -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Move VMX handling code out of the driver directory
On 12/21/2010 10:50 AM, Justin Clift wrote: On 22/12/2010, at 4:40 AM, Eric Blake wrote: snip 'git diff' is so awesome with renames :) Hmmm recently had a problem with it where it *insisted* that one of the 100+ files in a git repo I'd been messing with was a rename of a different file... even when it wasn't. (significantly different file contents) Didn't find an option to force it to recognise the file *wasn't* a rename, so from memory just had to leave it with it's incorrect rename entry in history. :( Surprisingly, git doesn't track renames (seriously, there's no metadata in the .git directory that tells you when a file was moved). It merely uses heuristics and a similarity index to guess when a rename exists. There are ways to tighten the similarity index (git diff -M90% instead of git diff -M), as well as disable similarity checking altogether (git diff -l1, and various settings in git config). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Move VMX handling code out of the driver directory
On 22/12/2010, at 4:57 AM, Eric Blake wrote: On 12/21/2010 10:50 AM, Justin Clift wrote: On 22/12/2010, at 4:40 AM, Eric Blake wrote: snip 'git diff' is so awesome with renames :) Hmmm recently had a problem with it where it *insisted* that one of the 100+ files in a git repo I'd been messing with was a rename of a different file... even when it wasn't. (significantly different file contents) Didn't find an option to force it to recognise the file *wasn't* a rename, so from memory just had to leave it with it's incorrect rename entry in history. :( Surprisingly, git doesn't track renames (seriously, there's no metadata in the .git directory that tells you when a file was moved). It merely uses heuristics and a similarity index to guess when a rename exists. There are ways to tighten the similarity index (git diff -M90% instead of git diff -M), as well as disable similarity checking altogether (git diff -l1, and various settings in git config). Interesting. Thanks, learn something every day. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] API break from the VMware player driver
All, I'll preface this by saying that I'm not 100% sure I'm correct. But I still think there may be an API break that was introduced with the VMware player driver. In include/libvirt/virterror.h, VIR_FROM_VMWARE was added to the *middle* of the enum for virErrorDomain. If any clients of libvirt happen to be using hardcoded numbers, then they will now have the wrong number for everything after VIR_FROM_VMWARE. Looking back through the history of virErrorDomain, it doesn't seem like this is the first time this has happened. What's the thinking with virErrorDomain? Part of the API, or up to the clients to properly use the named enums? -- Chris Lalancette -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] command: avoid hanging on daemon processes
* src/util/command.c (virCommandRun): Don't capture output on daemons. * tests/commandtest.c (test18): Expose the bug. Reported by Laine Stump. --- Even though 'test4' in commandtest created a daemon, the daemon exits rather quickly, so that no one noticed the problem. And the existing qemu daemon use of virCommand was supplying log file descriptors, rather than letting virCommand capture output. I've verified that with just the tests/ changes, that the testsuite fails (termination due to SIGALRM). src/util/command.c | 24 ++-- tests/commandtest.c | 42 ++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index f9d475e..abd2dc4 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1008,17 +1008,21 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) } /* If caller hasn't requested capture of stdout/err, then capture - * it ourselves so we can log it. + * it ourselves so we can log it. But the intermediate child for + * a daemon has no expected output, and we don't want our + * capturing pipes passed on to the daemon grandchild. */ -if (!cmd-outfdptr) { -cmd-outfdptr = cmd-outfd; -cmd-outbuf = outbuf; -string_io = true; -} -if (!cmd-errfdptr) { -cmd-errfdptr = cmd-errfd; -cmd-errbuf = errbuf; -string_io = true; +if (!(cmd-flags VIR_EXEC_DAEMON)) { +if (!cmd-outfdptr) { +cmd-outfdptr = cmd-outfd; +cmd-outbuf = outbuf; +string_io = true; +} +if (!cmd-errfdptr) { +cmd-errfdptr = cmd-errfd; +cmd-errbuf = errbuf; +string_io = true; +} } if (virCommandRunAsync(cmd, NULL) 0) { diff --git a/tests/commandtest.c b/tests/commandtest.c index 333dd4d..38a7816 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -668,6 +668,47 @@ cleanup: return ret; } +/* + * Run long-running daemon, to ensure no hang. + */ +static int test18(const void *unused ATTRIBUTE_UNUSED) +{ +virCommandPtr cmd = virCommandNewArgList(sleep, 100, NULL); +char *pidfile = virFilePid(abs_builddir, commandhelper); +pid_t pid; +int ret = -1; + +if (!pidfile) +goto cleanup; + +virCommandSetPidFile(cmd, pidfile); +virCommandDaemonize(cmd); + +alarm(5); +if (virCommandRun(cmd, NULL) 0) { +virErrorPtr err = virGetLastError(); +printf(Cannot run child %s\n, err-message); +goto cleanup; +} +alarm(0); + +if (virFileReadPid(abs_builddir, commandhelper, pid) != 0) { +printf(cannot read pidfile\n); +goto cleanup; +} +while (kill(pid, SIGINT) != -1) +usleep(100*1000); + +ret = 0; + +cleanup: +virCommandFree(cmd); +unlink(pidfile); +VIR_FREE(pidfile); +return ret; +} + + static int mymain(int argc, char **argv) { @@ -732,6 +773,7 @@ mymain(int argc, char **argv) DO_TEST(test15); DO_TEST(test16); DO_TEST(test17); +DO_TEST(test18); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.3.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] API break from the VMware player driver
On 12/21/2010 11:34 AM, Chris Lalancette wrote: All, I'll preface this by saying that I'm not 100% sure I'm correct. But I still think there may be an API break that was introduced with the VMware player driver. In include/libvirt/virterror.h, VIR_FROM_VMWARE was added to the *middle* of the enum for virErrorDomain. If any clients of libvirt happen to be using hardcoded numbers, then they will now have the wrong number for everything after VIR_FROM_VMWARE. virterror.h is public, we should fix it now before the next release, because these values are passed over the wire in RPC calls, and if the server on one machine uses different values than the client on another machine, then the client can misinterpret the error values. Looking back through the history of virErrorDomain, it doesn't seem like this is the first time this has happened. What's the thinking with virErrorDomain? I just glanced at recent changes; we broke virErrorNumber in March 2010 with commit 46e9b0f, but the last time we broke virErrorDomain was commit f163895 in Mar 2008. (Most other commits that touched mid-enum were just fixing typos or spacing). I don't know what to do about those two API breakages. Part of the API, or up to the clients to properly use the named enums? Part of the API, I'm afraid. See the tail end of: https://www.redhat.com/archives/libvir-list/2010-December/msg00617.html +/* NB. Fields code, domain and level are really enums. The + * numeric value should remain compatible between libvirt and + * libvirtd. This means, no changing or reordering the enums as + * defined in virterror.h (but we don't do that anyway, for separate + * ABI reasons). + */ +struct virNetMessageError { +int code; +int domain; -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] command: avoid hanging on daemon processes
On 12/21/2010 01:54 PM, Eric Blake wrote: * src/util/command.c (virCommandRun): Don't capture output on daemons. * tests/commandtest.c (test18): Expose the bug. Reported by Laine Stump. --- Even though 'test4' in commandtest created a daemon, the daemon exits rather quickly, so that no one noticed the problem. And the existing qemu daemon use of virCommand was supplying log file descriptors, rather than letting virCommand capture output. I've verified that with just the tests/ changes, that the testsuite fails (termination due to SIGALRM). src/util/command.c | 24 ++-- tests/commandtest.c | 42 ++ 2 files changed, 56 insertions(+), 10 deletions(-) ACK. This works properly for the case where it didn't work previously. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] API break from the VMware player driver
2010/12/21 Eric Blake ebl...@redhat.com: On 12/21/2010 11:34 AM, Chris Lalancette wrote: All, I'll preface this by saying that I'm not 100% sure I'm correct. But I still think there may be an API break that was introduced with the VMware player driver. In include/libvirt/virterror.h, VIR_FROM_VMWARE was added to the *middle* of the enum for virErrorDomain. If any clients of libvirt happen to be using hardcoded numbers, then they will now have the wrong number for everything after VIR_FROM_VMWARE. virterror.h is public, we should fix it now before the next release, because these values are passed over the wire in RPC calls, and if the server on one machine uses different values than the client on another machine, then the client can misinterpret the error values. We might explicit write out all values of the enum members. That'll make it more obvious that adding in the middle is a bad idea. Can we add a make syntax-check rule to avoid future breakage, like we have for the XDR protocol? PS: Sorry, for not noticing this during review of the VMware driver. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] command: avoid hanging on daemon processes
On 12/21/2010 12:40 PM, Laine Stump wrote: On 12/21/2010 01:54 PM, Eric Blake wrote: * src/util/command.c (virCommandRun): Don't capture output on daemons. * tests/commandtest.c (test18): Expose the bug. Reported by Laine Stump. --- Even though 'test4' in commandtest created a daemon, the daemon exits rather quickly, so that no one noticed the problem. And the existing qemu daemon use of virCommand was supplying log file descriptors, rather than letting virCommand capture output. I've verified that with just the tests/ changes, that the testsuite fails (termination due to SIGALRM). src/util/command.c | 24 ++-- tests/commandtest.c | 42 ++ 2 files changed, 56 insertions(+), 10 deletions(-) ACK. This works properly for the case where it didn't work previously. Now pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] maint: avoid space-tab
* daemon/Makefile.am: Avoid spurious space before tabs. * src/Makefile.am: Likewise. * examples/dominfo/Makefile.am: Likewise. * examples/domsuspend/Makefile.am: Likewise. * tools/Makefile.am: Likewise. * src/datatypes.h (VIR_CONNECT_MAGIC): Likewise. * src/internal.h (TODO): Likewise. * src/qemu/qemu_monitor.h (QEMU_MONITOR_MIGRATE): Likewise. * src/xen/xen_hypervisor.c (XEN_V2_OP_GETAVAILHEAP): Likewise. * src/xen/xs_internal.h: Likewise. --- Pushing this under the trivial rule - I've got my editor set up to highlight suspicious spacing, and when I noticed it in one file, I decided to scrub the rest of the source tree. There are also space-tab instances in a binary blob and in docs/api_extension (where there are literal patchsets containing doc changes that end up creating space-tab), but those instances obviously should not be changed. daemon/Makefile.am |4 ++-- examples/dominfo/Makefile.am|3 ++- examples/domsuspend/Makefile.am |3 ++- src/Makefile.am |4 ++-- src/datatypes.h |4 ++-- src/internal.h |2 +- src/qemu/qemu_monitor.h |2 +- src/xen/xen_hypervisor.c|2 +- src/xen/xs_internal.h | 14 +++--- tools/Makefile.am |6 +++--- 10 files changed, 23 insertions(+), 21 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 72778e5..3ffb7be 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -90,8 +90,8 @@ libvirtd_CFLAGS = \ -DQEMUD_PID_FILE=\$(QEMUD_PID_FILE)\ \ -DREMOTE_PID_FILE=\$(REMOTE_PID_FILE)\ -libvirtd_LDFLAGS = \ - $(WARN_CFLAGS) \ +libvirtd_LDFLAGS = \ + $(WARN_CFLAGS) \ $(COVERAGE_LDFLAGS) libvirtd_LDADD = \ diff --git a/examples/dominfo/Makefile.am b/examples/dominfo/Makefile.am index 2913e5b..678de68 100644 --- a/examples/dominfo/Makefile.am +++ b/examples/dominfo/Makefile.am @@ -1,6 +1,7 @@ INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -...@srcdir@/include -LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la $(COVERAGE_LDFLAGS) +LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \ + $(COVERAGE_LDFLAGS) noinst_PROGRAMS=info1 diff --git a/examples/domsuspend/Makefile.am b/examples/domsuspend/Makefile.am index 14b4205..2c277a4 100644 --- a/examples/domsuspend/Makefile.am +++ b/examples/domsuspend/Makefile.am @@ -1,6 +1,7 @@ INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -...@srcdir@/include -LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la $(COVERAGE_LDFLAGS) +LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \ + $(COVERAGE_LDFLAGS) noinst_PROGRAMS=suspend diff --git a/src/Makefile.am b/src/Makefile.am index fe34b7b..10c3c7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -254,7 +254,7 @@ OPENVZ_DRIVER_SOURCES = \ openvz/openvz_conf.c openvz/openvz_conf.h \ openvz/openvz_driver.c openvz/openvz_driver.h -VMWARE_DRIVER_SOURCES =\ +VMWARE_DRIVER_SOURCES =\ vmware/vmware_driver.c vmware/vmware_driver.h \ vmware/vmware_conf.c vmware/vmware_conf.h @@ -266,7 +266,7 @@ VBOX_DRIVER_SOURCES = \ vbox/vbox_V3_1.c vbox/vbox_CAPI_v3_1.h \ vbox/vbox_V3_2.c vbox/vbox_CAPI_v3_2.h -VBOX_DRIVER_EXTRA_DIST = \ +VBOX_DRIVER_EXTRA_DIST = \ vbox/vbox_tmpl.c vbox/README\ vbox/vbox_MSCOMGlue.c vbox/vbox_MSCOMGlue.h \ vbox/vbox_XPCOMCGlue.c vbox/vbox_XPCOMCGlue.h diff --git a/src/datatypes.h b/src/datatypes.h index bbeb7cf..07fa582 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -1,7 +1,7 @@ /* * datatypes.h: management of structs for public data types * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -34,7 +34,7 @@ * magic value used to protect the API when pointers to connection structures * are passed down by the uers. */ -# define VIR_CONNECT_MAGIC 0x4F23DEAD +# define VIR_CONNECT_MAGIC 0x4F23DEAD # define VIR_IS_CONNECT(obj) ((obj) (obj)-magic==VIR_CONNECT_MAGIC) diff --git a/src/internal.h b/src/internal.h index 8473c3c..038b862 100644 --- a/src/internal.h +++ b/src/internal.h @@ -200,7 +200,7 @@ * * macro to flag
[libvirt] [PATCH] Call initgroups for qemu's uid prior to exec
This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406 If qemu is run as a different uid, it has been unable to access mode 0660 files that are owned by a different user, but with a group that the qemu is a member of (aside from the one group listed in the passwd file). initgroups will change the group membership of the process (and its children) to match the new uid. --- src/qemu/qemu_security_dac.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index 55dc0c6..2e60aec 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -12,6 +12,8 @@ #include sys/types.h #include sys/stat.h #include fcntl.h +#include pwd.h +#include grp.h #include qemu_security_dac.h #include qemu_conf.h @@ -558,6 +560,30 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } if (driver-user) { +struct passwd pwd, *pwd_result; +char *buf = NULL; +size_t bufsize = 16384; + +if (VIR_ALLOC_N(buf, bufsize) 0) { +virReportOOMError(); +return -1; +} +getpwuid_r(driver-user, pwd, buf, bufsize, pwd_result); +if (pwd_result == NULL) { +virReportSystemError(errno, + _(cannot getpwuid_r(%d)), driver-user); +VIR_FREE(buf); +return -1; +} +if (initgroups(pwd.pw_name, pwd.pw_gid) != 0) { +virReportSystemError(errno, + _(cannot initgroups(\%s\, %d)), + pwd.pw_name, pwd.pw_gid); +VIR_FREE(buf); +return -1; +} +VIR_FREE(buf); + if (setreuid(driver-user, driver-user) 0) { virReportSystemError(errno, _(cannot change to '%d' user), @@ -566,6 +592,7 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } + return 0; } -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] New virSocketAddr utility functions
On 12/20/2010 06:40 PM, Eric Blake wrote: On 12/20/2010 01:03 AM, Laine Stump wrote: virSocketPrefixToNetmask: Given a 'prefix', which is the number of 1 bits in a netmask, fill in a virSocketAddr object with a netmask as an IP address (IPv6 or IPv4). virSocketAddrMask: Mask off the host bits in one virSocketAddr according to the netmask in another virSocketAddr. virSocketAddrMaskByPrefix, Mask off the host bits in a virSocketAddr according to a prefix (number of 1 bits in netmask). VIR_SOCKET_FAMILY: return the family of a virSocketAddr All sound quite useful. +if (addr-data.stor.ss_family == AF_INET6) { +int ii; +for (ii = 0; ii 4; ii++) +addr-data.inet6.sin6_addr.s6_addr32[ii] += netmask-data.inet6.sin6_addr.s6_addr32[ii]; Not portable. Nothing standardizes the existence of s6_addr32[4], and not all platforms provide this convenience alias. Instead, you'll have to iterate over s6_addr[16] bytes. Fixed. +int +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix) s/int prefix/unsigned/ Okay, I've changed all of these (except for return values which are prefix - those will remain int so that -1 can be returned for error). +{ +virSocketAddr netmask; + +if (virSocketAddrPrefixToNetmask(prefix,netmask, + addr-data.stor.ss_family) 0) +return -1; Avoid code duplication; use virSocketAddrMask to do the masking. Right. Not sure why I didn't do that to begin with. +int virSocketAddrPrefixToNetmask(int prefix, For consistency with the rest of the file, put a newline after the return type and start virSocketAddrPrefixToNetmask in the first column. Again, use unsigned int for prefix. Yep. I meant to put the return type on a separate line everywhere, but this one slipped. +if (family == AF_INET) { +int ip = 0; + +if (prefix 0 || prefix 32) +goto error; + +while (prefix-- 0) { +ip= 1; +ip |= 0x8000; Bit-wise loops are slow compared to direct computation. Why not just: if (prefix == 0) ip = 0; else ip = ~((1 (32 - prefix)) - 1); Or ip = prefix ? ~((1 (32 - prefix)) - 1) : 0; :-) +} else if (family == AF_INET6) { +int ii = 0; + +if (prefix 128) Technically, we could use (CHAR_BIT * sizeof netmask-data.inet6.sin6_addr.s6_addr) instead of 128, and so forth, but the magic numbers used in this function aren't too hard to see without having to hide them behind named constants, so I'm fine with keeping 128. +goto error; Another argument to make prefix unsigned, since you didn't check for negative values. + +while (prefix= 8) { +/* do as much as possible an entire byte at a time */ +netmask-data.inet6.sin6_addr.s6_addr[ii++] = 0xff; +prefix -= 8; +} okay. +while (prefix-- 0) { +/* final partial byte one bit at a time */ +netmask-data.inet6.sin6_addr.s6_addr[ii]= 1; +netmask-data.inet6.sin6_addr.s6_addr[ii] |= 0x80; +} +ii++; Given your assumption that you are not starting from an initialized value, this loop ends up putting garbage in the low half of the byte. Fix that, and avoid the bitwise loop at the same time, by replacing those six lines with: if (prefix 0) { netmask-data.inet6.sin6_addr.s6_addr[ii++] = ~((1 (8 - prefix)) - 1); } Done. Thanks for the thorough review. v2 coming up! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix cluster resource lookup when connecting to a vCenter
2010/12/20 Eric Blake ebl...@redhat.com: On 12/20/2010 12:04 PM, Matthias Bolte wrote: Connecting to a ESX(i) server that is part of a cluster failed when the connection also involved a vCenter. Accept ClusterComputeResource type in addition to ComputeResource type in the object lookup function. Reported by Guillaume Le Louët. --- src/esx/esx_vi.c | 13 - 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 74a2a42..482a118 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -538,7 +538,7 @@ esxVI_Context_LookupObjectsByPath(esxVI_Context *ctx, goto cleanup; } - /* Lookup ComputeResource */ + /* Lookup (Cluster)ComputeResource */ ACK. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Move VMX handling code out of the driver directory
2010/12/21 Eric Blake ebl...@redhat.com: On 12/21/2010 09:03 AM, Matthias Bolte wrote: Now the VMware driver doesn't depend on the ESX driver anymore. Add a WITH_VMX option that depends on WITH_ESX and WITH_VMWARE. Also add a libvirt_vmx.syms file. Move some escaping functions from esx_util.c to vmx.c. Adapt the test suite, ESX and VMware driver to the new code layout. --- a/src/esx/esx_driver.c @@ -88,7 +88,7 @@ struct _esxVMX_Data { * Firstly this functions checks if the given file name contains a separator. * If it doesn't then the referenced file is in the same directory as the .vmx * file. The datastore name and directory of the .vmx file are passed to this - * function via the opaque paramater by the caller of esxVMX_ParseConfig. + * function via the opaque paramater by the caller of virVMXParseConfig. As long as you're touching this line, s/paramater/parameter/ One of my typical typos, fixed now :) +++ b/src/libvirt_vmx.syms @@ -0,0 +1,23 @@ +# +# These symbols are dependent upon --with-esx via WITH_ESX or --with-vmware via WITH_VMWARE. +# + +# vmx.h +virVMXConvertToUTF8; +virVMXEscapeHex; +virVMXUnescapeHex; +virVMXParseConfig; Do you want to sort this list? I intended to do so but forgot about it before posting. It's sorted now. diff --git a/src/esx/esx_vmx.c b/src/vmx/vmx.c similarity index 79% rename from src/esx/esx_vmx.c rename to src/vmx/vmx.c index 5cbb835..6e3e9af 100644 'git diff' is so awesome with renames :) @@ -454,19 +456,23 @@ def-parallels[0]... */ -#define VIR_FROM_THIS VIR_FROM_ESX +#define VIR_FROM_THIS VIR_FROM_NONE Bit of a shame that we lose information here, but not the end of the world. ACK with the spelling and sorting nits addressed. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vmware: Fix undefine symbol with loadable drivers enabled
All other drivers are explicitly linked to gnulib. The VMware driver lacked this, resulting in mdir_name being an undefine symbol. Explicitly link the VMware driver to gnulib to fix this. --- src/Makefile.am |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 607e391..7ecd3e0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -633,6 +633,7 @@ endif libvirt_driver_vmware_la_CFLAGS = \ -...@top_srcdir@/src/conf -...@top_srcdir@/src/vmx if WITH_DRIVER_MODULES +libvirt_driver_vmware_la_LIBADD = ../gnulib/lib/libgnu.la libvirt_driver_vmware_la_LDFLAGS = -module -avoid-version endif libvirt_driver_vmware_la_SOURCES = $(VMWARE_DRIVER_SOURCES) -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Call initgroups for qemu's uid prior to exec
On 12/21/2010 01:45 PM, Laine Stump wrote: This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406 If qemu is run as a different uid, it has been unable to access mode 0660 files that are owned by a different user, but with a group that the qemu is a member of (aside from the one group listed in the passwd file). initgroups will change the group membership of the process (and its children) to match the new uid. --- src/qemu/qemu_security_dac.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index 55dc0c6..2e60aec 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -12,6 +12,8 @@ #include sys/types.h #include sys/stat.h #include fcntl.h +#include pwd.h +#include grp.h #include qemu_security_dac.h #include qemu_conf.h @@ -558,6 +560,30 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } if (driver-user) { +struct passwd pwd, *pwd_result; +char *buf = NULL; +size_t bufsize = 16384; qemu_driver.c sets this to 1024*1024. Will that matter? For that matter, can't you provide this functionality in a single .c file so that both qemudOpenAsUID and qemuSecurityDACSetProcessLabel can share the benefits of common code? That refactoring probably deserves a v2. @@ -566,6 +592,7 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } + return 0; Spurious whitespace change. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
On 12/20/2010 06:52 PM, Eric Blake wrote: On 12/20/2010 01:03 AM, Laine Stump wrote: Later patches will add the possibility to define a network's netmask as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to deal with definition of both kinds (prefix or netmask), add two new functions: virNetworkDefNetmask: return a copy of the netmask into a virSocketAddr. If no netmask was specified in the XML, create a default netmask based on the network class of the virNetworkDef's IP address. virNetworkDefPrefix: return the netmask as numeric prefix (or the default prefix for the network class of the virNetworkDef's IP address, if no netmask was specified in the XML) What happens if the user specifies a netmask in the XML that is non-contiguous (bad practice, but some routers do allow it)? If that's the case, +/* return number of 1 bits in netmask for the network's ipAddress, + * or -1 on error + */ +int virNetworkDefPrefix(const virNetworkDefPtr def) Should this return different values, such as -1 if network class not determined, and -2 if netmask was specified but non-contiguous? Or do callers not care? +{ +if (VIR_SOCKET_HAS_ADDR(def-netmask)) { +return virSocketGetNumNetmaskBits(def-netmask); [hmm, back to my theme of preferring direct operations over bitwise loops, I notice that the existing GetNumNetmaksBits implementation could be independently optimized] +} else if (VIR_SOCKET_IS_FAMILY(def-ipAddress, AF_INET)) { +/* return the natural prefix for the network's ip address */ +int addr = ntohl(def-ipAddress.data.inet4.sin_addr.s_addr); +if (IN_CLASSA(addr)) Where is this defined? Oh, I found it - innetinet/in.h, but only as a Linux extension. Since POSIX doesn't require it to exist, you'll need to take care to provide fallback definitions for this macro and its friends. If the raw numbers have to be in there anyway, I'd rather just do it by hand for all platforms. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vmware: Fix undefine symbol with loadable drivers enabled
On 12/21/2010 02:51 PM, Matthias Bolte wrote: All other drivers are explicitly linked to gnulib. The VMware driver lacked this, resulting in mdir_name being an undefine symbol. That's the minimum impact on glibc. The impact on other systems like mingw would be even bigger, due to lots of missing rpl_ functions. Explicitly link the VMware driver to gnulib to fix this. --- src/Makefile.am |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 607e391..7ecd3e0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -633,6 +633,7 @@ endif libvirt_driver_vmware_la_CFLAGS = \ -...@top_srcdir@/src/conf -...@top_srcdir@/src/vmx if WITH_DRIVER_MODULES +libvirt_driver_vmware_la_LIBADD = ../gnulib/lib/libgnu.la ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] error: avoid API breakage with vmware
Fix glitch in commit cddd2a06 (thankfully post-0.8.6, so no released version has the glitch). Document and try to workaround glitch in commit 46e9b0f (in 0.8.0, which invalidated 6 virErrorNumber dating back to 0.7.1). Thankfully, my audit did not find any other glitches until pre-0.1.0 days. I'm not sure how to add a syntax-check off the top of my head, but hopefully the explicit numbering will make people think twice about renumbering in the future. * include/libvirt/virterror.h (virErrorDomain): Avoid inserting new values in the middle, and add explicit numbering to help avoid this in the future. (virErrorNumber): Add explicit numbering, and document the snafu. * src/remote/remote_driver.c (remoteIO): Compensate for the snafu. --- include/libvirt/virterror.h | 266 -- src/remote/remote_driver.c | 30 +- 2 files changed, 181 insertions(+), 115 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index a1f88f4..0e59c0e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copy: Copyright (C) 2006 Red Hat, Inc. + * Copy: Copyright (C) 2006, 2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -34,49 +34,51 @@ typedef enum { /** * virErrorDomain: * - * Indicates where an error may have come from + * Indicates where an error may have come from. This should remain + * stable, with all additions placed at the end since libvirt 0.1.0. */ typedef enum { VIR_FROM_NONE = 0, -VIR_FROM_XEN, /* Error at Xen hypervisor layer */ -VIR_FROM_XEND, /* Error at connection with xend daemon */ -VIR_FROM_XENSTORE, /* Error at connection with xen store */ -VIR_FROM_SEXPR,/* Error in the S-Expression code */ -VIR_FROM_XML, /* Error in the XML code */ -VIR_FROM_DOM, /* Error when operating on a domain */ -VIR_FROM_RPC, /* Error in the XML-RPC code */ -VIR_FROM_PROXY,/* Error in the proxy code */ /* unused since 0.8.6 */ -VIR_FROM_CONF, /* Error in the configuration file handling */ -VIR_FROM_QEMU, /* Error at the QEMU daemon */ -VIR_FROM_NET, /* Error when operating on a network */ -VIR_FROM_TEST, /* Error from test driver */ -VIR_FROM_REMOTE, /* Error from remote driver */ -VIR_FROM_OPENVZ,/* Error from OpenVZ driver */ -VIR_FROM_VMWARE,/* Error from VMware driver */ -VIR_FROM_XENXM,/* Error at Xen XM layer */ -VIR_FROM_STATS_LINUX,/* Error in the Linux Stats code */ -VIR_FROM_LXC, /* Error from Linux Container driver */ -VIR_FROM_STORAGE, /* Error from storage driver */ -VIR_FROM_NETWORK, /* Error from network config */ -VIR_FROM_DOMAIN,/* Error from domain config */ -VIR_FROM_UML, /* Error at the UML driver */ -VIR_FROM_NODEDEV, /* Error from node device monitor */ -VIR_FROM_XEN_INOTIFY,/* Error from xen inotify layer */ -VIR_FROM_SECURITY, /* Error from security framework */ -VIR_FROM_VBOX, /* Error from VirtualBox driver */ -VIR_FROM_INTERFACE, /* Error when operating on an interface */ -VIR_FROM_ONE, /* Error from OpenNebula driver */ -VIR_FROM_ESX, /* Error from ESX driver */ -VIR_FROM_PHYP, /* Error from IBM power hypervisor */ -VIR_FROM_SECRET,/* Error from secret storage */ -VIR_FROM_CPU, /* Error from CPU driver */ -VIR_FROM_XENAPI,/* Error from XenAPI */ -VIR_FROM_NWFILTER, /* Error from network filter driver */ -VIR_FROM_HOOK, /* Error from Synchronous hooks */ -VIR_FROM_DOMAIN_SNAPSHOT, /* Error from domain snapshot */ -VIR_FROM_AUDIT, /* Error from auditing subsystem */ -VIR_FROM_SYSINFO, /* Error from sysinfo/SMBIOS */ -VIR_FROM_STREAMS, /* Error from I/O streams */ +VIR_FROM_XEN = 1, /* Error at Xen hypervisor layer */ +VIR_FROM_XEND = 2, /* Error at connection with xend daemon */ +VIR_FROM_XENSTORE = 3, /* Error at connection with xen store */ +VIR_FROM_SEXPR = 4,/* Error in the S-Expression code */ +VIR_FROM_XML = 5, /* Error in the XML code */ +VIR_FROM_DOM = 6, /* Error when operating on a domain */ +VIR_FROM_RPC = 7, /* Error in the XML-RPC code */ +VIR_FROM_PROXY = 8,/* Error in the proxy code; unused since + 0.8.6 */ +VIR_FROM_CONF = 9, /* Error in the configuration file handling */ +VIR_FROM_QEMU = 10,/* Error at the QEMU daemon */ +VIR_FROM_NET = 11, /* Error when operating on a network */ +VIR_FROM_TEST = 12,/* Error from test driver */ +VIR_FROM_REMOTE = 13, /* Error from remote driver */ +
Re: [libvirt] [PATCH] vmware: Fix undefine symbol with loadable drivers enabled
2010/12/21 Eric Blake ebl...@redhat.com: On 12/21/2010 02:51 PM, Matthias Bolte wrote: All other drivers are explicitly linked to gnulib. The VMware driver lacked this, resulting in mdir_name being an undefine symbol. That's the minimum impact on glibc. The impact on other systems like mingw would be even bigger, due to lots of missing rpl_ functions. Explicitly link the VMware driver to gnulib to fix this. --- src/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 607e391..7ecd3e0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -633,6 +633,7 @@ endif libvirt_driver_vmware_la_CFLAGS = \ -...@top_srcdir@/src/conf -...@top_srcdir@/src/vmx if WITH_DRIVER_MODULES +libvirt_driver_vmware_la_LIBADD = ../gnulib/lib/libgnu.la ACK. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bridge_driver: use conffile for dnsmasq if it exists
By default dnsmasq is spawned with option --conf-file= which disables reading of global configuration file -- this is fine for most situations. This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named network_name-dnsmasq.conf in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before. Patch should be applied after my earlier one. --- src/network/bridge_driver.c | 26 +++--- 1 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f2857b4..702ec95 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -391,6 +391,7 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, static int networkBuildDnsmasqArgv(virNetworkObjPtr network, const char *pidfile, +const char *conffile, virCommandPtr cmd) { int r, ret = -1; int nbleases = 0; @@ -428,8 +429,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, virCommandAddArgPair(cmd, --pid-file, pidfile); -/* *no* conf file */ -virCommandAddArgList(cmd, --conf-file=, , NULL); +/* if conf file exists use it */ +if (virFileExists(conffile)) +virCommandAddArgPair(cmd, --conf-file, conffile); +else +virCommandAddArgList(cmd, --conf-file=, , NULL); /* * XXX does not actually work, due to some kind of @@ -528,35 +532,41 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) virCommandPtr cmd = NULL; char *pidfile = NULL; int ret = -1, err; +char *conffile = NULL; network-dnsmasqPid = -1; if (!VIR_SOCKET_IS_FAMILY(network-def-ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot start dhcp daemon without IPv4 address for server)); -goto cleanup2; +goto cleanup3; } if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _(cannot create directory %s), NETWORK_PID_DIR); -goto cleanup2; +goto cleanup3; } if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(err, _(cannot create directory %s), NETWORK_STATE_DIR); -goto cleanup2; +goto cleanup3; } if (!(pidfile = virFilePid(NETWORK_PID_DIR, network-def-name))) { virReportOOMError(); +goto cleanup3; +} + +if (virAsprintf(conffile, DNSMASQ_STATE_DIR /%s-dnsmasq.conf, network-def-name) 0) { +virReportOOMError(); goto cleanup2; } cmd = virCommandNew(DNSMASQ); -if (networkBuildDnsmasqArgv(network, pidfile, cmd) 0) { +if (networkBuildDnsmasqArgv(network, pidfile, conffile, cmd) 0) { goto cleanup1; } @@ -577,9 +587,11 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) ret = 0; cleanup1: -VIR_FREE(pidfile); virCommandFree(cmd); +VIR_FREE(conffile); cleanup2: +VIR_FREE(pidfile); +cleanup3: return ret; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bridge_driver: cleanup improvements in dhcpStartDhcpDaemon()
Run VIR_FREE only for non-NULL pointers. --- src/network/bridge_driver.c | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b0834ae..f2857b4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -534,34 +534,34 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) if (!VIR_SOCKET_IS_FAMILY(network-def-ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot start dhcp daemon without IPv4 address for server)); -goto cleanup; +goto cleanup2; } if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _(cannot create directory %s), NETWORK_PID_DIR); -goto cleanup; +goto cleanup2; } if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(err, _(cannot create directory %s), NETWORK_STATE_DIR); -goto cleanup; +goto cleanup2; } if (!(pidfile = virFilePid(NETWORK_PID_DIR, network-def-name))) { virReportOOMError(); -goto cleanup; +goto cleanup2; } cmd = virCommandNew(DNSMASQ); if (networkBuildDnsmasqArgv(network, pidfile, cmd) 0) { -goto cleanup; +goto cleanup1; } if (virCommandRun(cmd, NULL) 0) -goto cleanup; +goto cleanup1; /* * There really is no race here - when dnsmasq daemonizes, its @@ -573,12 +573,13 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) if (virFileReadPid(NETWORK_PID_DIR, network-def-name, network-dnsmasqPid) 0) -goto cleanup; +goto cleanup1; ret = 0; -cleanup: +cleanup1: VIR_FREE(pidfile); virCommandFree(cmd); +cleanup2: return ret; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/13] New virNetworkDef utility functions
On 12/21/2010 04:52 PM, Laine Stump wrote: On 12/20/2010 06:52 PM, Eric Blake wrote: On 12/20/2010 01:03 AM, Laine Stump wrote: Later patches will add the possibility to define a network's netmask as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to deal with definition of both kinds (prefix or netmask), add two new functions: virNetworkDefNetmask: return a copy of the netmask into a virSocketAddr. If no netmask was specified in the XML, create a default netmask based on the network class of the virNetworkDef's IP address. virNetworkDefPrefix: return the netmask as numeric prefix (or the default prefix for the network class of the virNetworkDef's IP address, if no netmask was specified in the XML) What happens if the user specifies a netmask in the XML that is non-contiguous (bad practice, but some routers do allow it)? If that's the case, Sorry, I accidentally hit send before I finished my thought... If someone wants to use a non-contiguous netmask, they can't use libvirt's virtual networks anyway, because iptables doesn't work with non-contiguous netmasks (netmask is specified as a prefix in all iptables commands - a later patch in this series takes advantage of that to simplify things). So I don't think it's something we need to be concerned about. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bridge_driver: cleanup improvements in dhcpStartDhcpDaemon()
2010/12/21 Paweł Krześniak pawel.krzesn...@gmail.com: Run VIR_FREE only for non-NULL pointers. --- src/network/bridge_driver.c | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b0834ae..f2857b4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -534,34 +534,34 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) if (!VIR_SOCKET_IS_FAMILY(network-def-ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot start dhcp daemon without IPv4 address for server)); - goto cleanup; + goto cleanup2; } ret = 0; -cleanup: +cleanup1: VIR_FREE(pidfile); virCommandFree(cmd); +cleanup2: return ret; } NACK as written, because you're complicating this function unnecessarily. It's totally fine to call VIR_FREE on a possible NULL pointer without checking, this is the common pattern in libvirt. Your additional label resembles the logic of if (pointer != NULL) { VIR_FREE(pointer); } This pattern in explicitly banned in libvirt by the make syntax-check rules and should to be simplified to VIR_FREE(pointer); as VIR_FREE (just like the normal free function) is safe to be called on a NULL pointer. If you really insists in avoiding the VIR_FREE call in the error path when the pointer is NULL, then you could just replace your goto cleanup2 by return -1, otherwise I suggest to leave this function as is, because there is nothing to fix here. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] API break from the VMware player driver
On Tue, Dec 21, 2010 at 08:50:04PM +0100, Matthias Bolte wrote: 2010/12/21 Eric Blake ebl...@redhat.com: On 12/21/2010 11:34 AM, Chris Lalancette wrote: All, I'll preface this by saying that I'm not 100% sure I'm correct. But I still think there may be an API break that was introduced with the VMware player driver. In include/libvirt/virterror.h, VIR_FROM_VMWARE was added to the *middle* of the enum for virErrorDomain. If any clients of libvirt happen to be using hardcoded numbers, then they will now have the wrong number for everything after VIR_FROM_VMWARE. virterror.h is public, we should fix it now before the next release, Absolutely ! BTW I will send a mail about next release suggestion later today, now that my move is over and I'm back online. because these values are passed over the wire in RPC calls, and if the server on one machine uses different values than the client on another machine, then the client can misinterpret the error values. We might explicit write out all values of the enum members. That'll make it more obvious that adding in the middle is a bad idea. agreed too, Can we add a make syntax-check rule to avoid future breakage, like we have for the XDR protocol? PS: Sorry, for not noticing this during review of the VMware driver. Well it happens, but yes an automated extra check for all enums in the public APIs would be good, I think this could be also managed with libvirt-api.xml extracted values, assuming we put the numbers in. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Distribute libvirt_vmx.syms
This fixes the build from a tarball and makes autobuild.sh work again. This should actually have been part of this earlier commit: esx: Move VMX handling code out of the driver directory 42b2f35d36a9e33f03e973130267c19cff910f2e Reported by Eric Blake. --- I pushed this under the (auto)build breaker rule :) Matthias src/Makefile.am |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 7ecd3e0..41d4b34 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1066,7 +1066,8 @@ EXTRA_DIST += \ libvirt_linux.syms \ libvirt_macvtap.syms \ libvirt_daemon.syms \ - libvirt_nwfilter.syms + libvirt_nwfilter.syms\ + libvirt_vmx.syms BUILT_SOURCES += libvirt.syms libvirt.def libvirt_qemu.def -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/13] Fix logging of failed iptables commands
On 12/20/2010 06:57 PM, Eric Blake wrote: On 12/20/2010 01:03 AM, Laine Stump wrote: The functions in iptables.c all return -1 on failure, but all their callers (which all happen to be in bridge_driver.c) assume that they are returning an errno, and the logging is done accordingly. This patch fixes all the error checking and logging to assume 0 is an error, and nothing else. --- src/network/bridge_driver.c | 183 +-- 1 files changed, 91 insertions(+), 92 deletions(-) Do any of the iptables.c functions guarantee that errno is a sane value when returning -1? There are only two possible reasons for any of the functions in iptables.c to fail: 1) out of memory, or 2) the exec of the iptables command fails or returns a non-0 status. In all OOM cases, a message has already been logged before returning, and it's likely the act of doing that reporting will trash errno (I haven't checked). (And by the time that happens, you're so hosed that it's not going to matter that a later error message overwrites that message or whatever). In the case of a problem exec'ing iptables, there could be a valid errno, or not (if the command just returned a non-0 exit code. At any rate, nobody has been looking at errno on return from the iptables functions; they've been attempting to interpret a -1 return code (placed into the local err) as an errno-like value, which simply isn't correct. This patch fixes that misconception. -virReportSystemError(err, - _(failed to add iptables rule to allow forwarding from '%s'), - network-def-bridge); +if (iptablesAddForwardAllowOut(driver-iptables, +network-def-ipAddress, +network-def-netmask, + network-def-bridge, + network-def-forwardDev) 0) { +networkReportError(VIR_ERR_SYSTEM_ERROR, + _(failed to add iptables rule to allow forwarding from '%s'), + network-def-bridge); or are we okay with this slightly less-informative message, if we happen to be ignoring a valid errno? Then again, given that the old code was using strerror(-1), which doesn't convey any information, we aren't really losing anything in practice from the old code by not displaying errno, even if iptables guaranteed that errno was useful. Therefore: ACK as-is. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Call initgroups for qemu's uid prior to exec
On 12/21/2010 04:52 PM, Eric Blake wrote: On 12/21/2010 01:45 PM, Laine Stump wrote: This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406 If qemu is run as a different uid, it has been unable to access mode 0660 files that are owned by a different user, but with a group that the qemu is a member of (aside from the one group listed in the passwd file). initgroups will change the group membership of the process (and its children) to match the new uid. --- src/qemu/qemu_security_dac.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index 55dc0c6..2e60aec 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -12,6 +12,8 @@ #includesys/types.h #includesys/stat.h #includefcntl.h +#includepwd.h +#includegrp.h #include qemu_security_dac.h #include qemu_conf.h @@ -558,6 +560,30 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } if (driver-user) { +struct passwd pwd, *pwd_result; +char *buf = NULL; +size_t bufsize = 16384; qemu_driver.c sets this to 1024*1024. Will that matter? The usage in qemu_driver.c re-uses a buffer that was already conveniently allocated for something else. This buffer is used to contain all the strings encountered in a single entry from /etc/passwd. The manpage for getpwuid_r() has example code to learn the maximum possible size for all the strings in one line, but that example falls back to 16384 if it fails to learn the system limit, saying this should be big enough for anything. For that matter, can't you provide this functionality in a single .c file so that both qemudOpenAsUID and qemuSecurityDACSetProcessLabel can share the benefits of common code? If I put it in a separate function, I won't feel so bad about adding in the extra code to detect the absolute max size to allocate. Of course, if I put it in a util file, we'll have to worry about making it compile on all you mother's favorite platforms, so... :-) That refactoring probably deserves a v2. Sure, I'll take a stab at it. @@ -566,6 +592,7 @@ qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } } + return 0; Spurious whitespace change. Oops. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2] storage: Ignore dangling symbol link for filesystem pool
于 2010年12月21日 22:40, Eric Blake 写道: On 12/20/2010 11:47 PM, Osier Yang wrote: a more efficient solution would be to check if errno is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents. I guess you mean stat, lstat will not work here, as it doesn't follow the *symbolic* link. what we need to do is to determine if the symbolic link is dangling, so use stat to update the patch, v3 send, thanks again. No, I really meant lstat(). If stat() would have failed because of a dangling symlink, then open() will fail for the same reasons. Therefore, check errno before lstat, and use the successful lstat as proof that a symlink is in place but that stat()ing the symlink would fail because it is dangling. if (open(f) 0) { if ((errno == ENOENT || errno == ELOOP) lstat(f, buf) == 0) { /* Dangling symlink, since lstat() passed but open failed. */ log message about ignored file return -2; } either an unrelated errno, like EACCES, or we got ENOENT because the file was deleted after readdir but before open/lstat error message about unaccessible file return -1; } Eric, thanks for the detailed knowledge.. - Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bridge_driver: use conffile for dnsmasq if it exists
On 12/21/2010 03:40 PM, Paweł Krześniak wrote: By default dnsmasq is spawned with option --conf-file= which disables reading of global configuration file -- this is fine for most situations. In fact, the libvirt policy is that it is essential to NOT allow the use of global configuration files - if it is worth changing, it is worth calling out directly in the XML directly. Why? Because if you run your guest today, then someone edits the global config file, and you run your guest tomorrow, you have no explainable reason logged in libvirt's generated qemu/dnsmasq/... command line that explains the difference in behavior, if those differences are hidden inside a global config file of an external tool. Furthermore, from the security perspective, sVirt requires that separate domains cannot share resources, and that should include common host config files. This patch adds possibility to run customized DNS/DHCP environment, by spawning dnsmasq with alternative configuration file if such file exists. This allows you to set any parameter described in dnsmasq(8). Configuration file is expected to be located in file named network_name-dnsmasq.conf in DNSMASQ_STATE_DIR directory. If configuration file doesn't exists dnsmasq is spawned as before. You'll want to wait for danpb or DV to comment, but I'm thinking this might be rejected, and that instead, we should consider addressing the issue of what dnsmasq parameters you want to affect, and how we can encode that into the libvirt XML without having to rely on an external dnsmasq conf file. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix memory leak in virsh
--- tools/virsh.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4e37f2d..8c123bb 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10935,8 +10935,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) if (tk == VSH_TK_ERROR) goto syntaxError; -if (tk != VSH_TK_ARG) +if (tk != VSH_TK_ARG) { +VIR_FREE(tkdata); break; +} if (cmd == NULL) { /* first token must be command name */ -- 1.7.3.1 -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Anyone interested in forming a libvirt docs team?
On 12/21/2010 05:50 PM, Justin Clift wrote: On 21/12/2010, at 10:32 PM, Zdenek Styblik wrote: Hello Justin, could you be more specific, please? How to put it. Your e-mail is too ambiguous for me :) Heh, no worries. I often talk in general terms first. :) The email response to Anthony, sent a few minutes ago, has more detail. I know there's a lot of demand for better documentation in libvirt, and other Open Source projects around have gotten together teams/groups of people who's main mission is to improving their documentation/manuals/learning materials (and so on). So, I reckon we see about doing the same thing for libvirt. Looks like there are people willing to pitch in, so I guess it's now a matter of figuring out what needs to be done, who's interested in doing what, and so on. If we need a libvirt-docs mailing list too, that can be done without too much effort, but not sure it's needed just yet. (open to suggestions :)) Hmmm, that email reply to Anthony should probably be copied to the wiki and we can expand on it from there. :) Is this the kind of detail you're after Zdenek? Regards and best wishes, Justin Clift Yes Justin, your reply to Anthony is just fine. Exactly where I wanted to push it, although I could be less ambiguous myself :) As for documentation in HTML. How about to write parser which would download pages from wiki, cut eg. menu, footer = garbage off, and replace it with whatever you want and like? I have to say I haven't seen documentation shipped with libvirt [heh?! :) ], but since you say it's HTML and Wiki output *is* HTML ... hm? Just an idea. Have a nice day, Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list