[libvirt] [PATCH] xen: Prevent updating device when attaching a device

[Osier Yang]

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

[Justin Clift]

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

[Jiri Denemark]

 * 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

[Jiri Denemark]

 * 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?

[Justin Clift]

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?

[Justin Clift]

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?

[Zdenek Styblik]

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?

[Anthony Davis]

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

[Eric Blake]

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

[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 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

[Eric Blake]

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

[Eric Blake]

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?

[Justin Clift]

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

[Eric Blake]

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?

[Justin Clift]

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

[Eric Blake]

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?

[Justin Clift]

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

[Eric Blake]

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

[Justin Clift]

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

[Eric Blake]

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

[Justin Clift]

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

[Chris Lalancette]

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

[Eric Blake]

* 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

[Eric Blake]

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

[Laine Stump]

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

[Matthias Bolte]

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

[Eric Blake]

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

[Eric Blake]

* 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

[Laine Stump]

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

[Laine Stump]

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

[Matthias Bolte]

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

[Matthias Bolte]

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

[Matthias Bolte]

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

[Eric Blake]

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

[Laine Stump]

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

[Eric Blake]

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

[Eric Blake]

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

[Matthias Bolte]

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

[Paweł Krześniak]

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()

[Paweł Krześniak]

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

[Laine Stump]

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()

[Matthias Bolte]

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

[Daniel Veillard]

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

[Matthias Bolte]

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

[Laine Stump]

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

[Laine Stump]

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

[Osier Yang]

于 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

[Eric Blake]

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

[Hu Tao]

---
 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?

[Zdenek Styblik]

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