Re: [libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs

2012-01-25 Thread Guan Nan Ren


- Original Message -
From: Eric Blake ebl...@redhat.com
To: Guan Nan Ren g...@redhat.com
Cc: libvir-list@redhat.com
Sent: Tuesday, January 24, 2012 8:56:30 PM
Subject: Re: [libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs

On 01/24/2012 05:07 AM, Guan Nan Ren wrote:
  static PyObject *
 +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,
 + PyObject *args) {
 
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainGetNumaParameters(domain, params, nparams, flags);
 +LIBVIRT_END_ALLOW_THREADS;
 
 Why are we getting the existing parameters, instead of just blindly
 constructing params based on args?  I think that's a waste of effort,
 considering that libvirt will already reject things if the user passes
 in unknown key names.

I finally realized _why_ we do it - that's because we want to pass the
correct types to libvirt.c, but python is not strongly typed.  That is,
if libvirt is expecting a particular named value to be
VIR_TYPED_PARAM_ULLONG, but the python code passes '1', we should be
able to properly convert that python value to C's 1ULL, rather than
rejecting the python code for using an incorrect type, since there is
not quite a notion of incorrect type in python.

There is another reason for this, we couldn't determine which 
one or more parameter will be set in case of multiple parameters, 
unless we hard-code the parameter name to compare. So the a little of overhead
caused by the loop and reassigning new value to given parameter based on 
arg is acceptable.

That means that either we hard-code the list of all known parameter
names and their type, or we make things done via a runtime query by
getting all parameters to learn their types before formatting the user's
settings back into something that libvirt will parse.

It may also mean that libvirt itself should be a bit nicer about things
- now that we have virTypedParam.c, maybe that function should be taught
to do some type conversions (if the user passes in VIR_TYPED_PARAM_INT,
but the code wanted VIR_TYPED_PARAM_ULLONG, libvirt could do the type
conversion on the user's behalf); if libvirt is made smarter, then the
python glue code might be simpler.  But there's still the issue of
back-compat - a newer python library talking to an older libvirt that
didn't do automatic type conversion would still be stuck needing to
query for types first.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: Emit bootindex even for direct boot

2012-01-25 Thread Jiri Denemark
On Tue, Jan 24, 2012 at 15:59:10 -0700, Eric Blake wrote:
 On 01/24/2012 03:53 PM, Jiri Denemark wrote:
  Direct boot (using kernel, initrd, and command line) is used by
  virt-install/virt-manager for network install. While any bootindex has
  no direct effect since -kernel is always first, we need it as a hint for
  SeaBIOS to present disks in the same order as they will be presented
  during normal boot.
  ---
   src/qemu/qemu_command.c |3 +--
   1 files changed, 1 insertions(+), 2 deletions(-)
  
  diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
  index aaccf62..9b69ad0 100644
  --- a/src/qemu/qemu_command.c
  +++ b/src/qemu/qemu_command.c
  @@ -4334,8 +4334,7 @@ qemuBuildCommandLine(virConnectPtr conn,
   if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
   int bootCD = 0, bootFloppy = 0, bootDisk = 0;
   
  -if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex) 
  
  -!def-os.kernel) {
  +if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || 
  emitBootindex)) {
 
 ACK.

Thanks, pushed.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] lxc: export container=lxc-libvirt for systemd

2012-01-25 Thread Daniel P. Berrange
On Tue, Jan 24, 2012 at 11:53:38AM -0700, Eric Blake wrote:
 Systemd detects containers based on whether they have
 an environment variable starting with 'container=lxc';
 using a longer name fits the expectations, while also
 allowing detection of who created the container.
 
 Requested by Lennart Poettering, in response to
 https://bugs.freedesktop.org/show_bug.cgi?id=45175
 
 * src/lxc/lxc_container.c (lxcContainerBuildInitCmd): Add another
 env-var.
 ---
  src/lxc/lxc_container.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index c1ec9c4..70f6908 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -1,5 +1,5 @@
  /*
 - * Copyright (C) 2008-2011 Red Hat, Inc.
 + * Copyright (C) 2008-2012 Red Hat, Inc.
   * Copyright (C) 2008 IBM Corp.
   *
   * lxc_container.c: file description
 @@ -124,6 +124,7 @@ static virCommandPtr 
 lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
 
  virCommandAddEnvString(cmd, PATH=/bin:/sbin);
  virCommandAddEnvString(cmd, TERM=linux);
 +virCommandAddEnvString(cmd, container=lxc-libvirt);
  virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr);
  virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name);
  if (vmDef-os.cmdline)

ACK, I had actually offered to do this before, but at the time
it was though having LIBVIRT_LXC_UUID was fine. Oh well.


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 0/3] Use guest agent to quiesce disks

2012-01-25 Thread Jiri Denemark
On Tue, Jan 24, 2012 at 21:21:32 +0100, Michal Privoznik wrote:
 Since we have qemu guest agent support in libvirt,
 we can start wiring up some things that GA already
 knows how to do. One of them is file system freeze
 and thaw. Domain snapshots can profit from this
 functionality.
 
 Michal Privoznik (3):
   qemu_agent: Create file system freeze and thaw functions
   snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
   virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag

ACK series.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 0/3] Use guest agent to quiesce disks

2012-01-25 Thread Michal Privoznik
On 25.01.2012 11:02, Jiri Denemark wrote:
 On Tue, Jan 24, 2012 at 21:21:32 +0100, Michal Privoznik wrote:
 Since we have qemu guest agent support in libvirt,
 we can start wiring up some things that GA already
 knows how to do. One of them is file system freeze
 and thaw. Domain snapshots can profit from this
 functionality.

 Michal Privoznik (3):
   qemu_agent: Create file system freeze and thaw functions
   snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
   virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
 
 ACK series.
 
 Jirka

Thanks, pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Memory leak on list_domains

2012-01-25 Thread Carlos Rodrigues
Hi,

I have some problems on my application with memory leak when call
list_domains method.

I'm using libvirt 0.8.3 and Sys::Virt 0.2.4 Perl Module.

Does anyone have any idea what's the problem?

Regards,

-- 
Carlos Rodrigues c...@eurotux.com
 Eurotux Informática, S.A. [http://eurotux.com]



signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol

2012-01-25 Thread Michal Privoznik
On 23.01.2012 14:30, Jiri Denemark wrote:
 We already provide ways to detect when a domain has been paused as a
 result of I/O error, but there was no way of getting the exact error or
 even the device that experienced it.  This new API may be used for both.
 ---
  include/libvirt/libvirt.h.in |   19 +++
  src/driver.h |6 
  src/libvirt.c|   53 
 ++
  src/libvirt_public.syms  |5 
  src/remote/remote_driver.c   |1 +
  src/remote/remote_protocol.x |   13 +-
  src/remote_protocol-structs  |9 +++
  7 files changed, 105 insertions(+), 1 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 958e5a6..2c3423a 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn,
 int interval,
 unsigned int count);
  
 +/**
 + * virDomainIoError:
 + *
 + * Disk I/O error.
 + */
 +typedef enum {
 +VIR_DOMAIN_IOERROR_NONE = 0, /* no error */
 +VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */
 +VIR_DOMAIN_IOERROR_UNSPEC   = 2, /* unspecified I/O error */

Just cosmetic, since this is expected to be extended one day, I'd like
to see _UNSPEC either at the beginning or at the end, but not in the
middle. However, the latter is not possible as it's API change.
But I can live with this thought.

 +
 +#ifdef VIR_ENUM_SENTINELS
 +VIR_DOMAIN_IOERROR_LAST
 +#endif
 +} virDomainIoError;
 +
 +int virDomainGetIoError(virDomainPtr dom,
 +const char *dev,
 +unsigned int flags);
 +
  #ifdef __cplusplus
  }
  #endif
 diff --git a/src/driver.h b/src/driver.h
 index 24636a4..1dd3760 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -794,6 +794,11 @@ typedef int
int *nparams,
unsigned int flags);
  
 +typedef int
 +(*virDrvDomainGetIoError)(virDomainPtr dom,
 +  const char *dev,
 +  unsigned int flags);
 +
  /**
   * _virDriver:
   *
 @@ -962,6 +967,7 @@ struct _virDriver {
  virDrvNodeSuspendForDuration nodeSuspendForDuration;
  virDrvDomainSetBlockIoTune domainSetBlockIoTune;
  virDrvDomainGetBlockIoTune domainGetBlockIoTune;
 +virDrvDomainGetIoError domainGetIoError;
  };
  
  typedef int
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 7b8adf7..99edca3 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -17882,3 +17882,56 @@ error:
  virDispatchError(dom-conn);
  return -1;
  }
 +
 +/**
 + * virDomainGetIoError:
 + * @dom: a domain object
 + * @dev: disk device to be inspected or NULL
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * The @disk parameter is either the device target (the dev attribute of
 + * target subelement), such as vda, or NULL, in which case all disks will 
 be
 + * inspected for errors. If only one disk experienced an I/O error, that 
 error
 + * will be returned. However, if there are more disks with I/O errors, this
 + * function will fail and return -2, requiring the caller to check every
 + * device explicitly.
 + *
 + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 if
 + * the function fails to do its job, the I/O error (virDomainIoError) 
 observed
 + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is returned
 + * when there is no error on the disk.
 + */
 +int
 +virDomainGetIoError(virDomainPtr dom,
 +const char *dev,
 +unsigned int flags)

Do we want to narrow this only for disks? Maybe some day hypervisors
will report IO errors on other devices as well (haven't tried to find
out if they do now days, though). However, if we restrict this only for
disks, I'd change the name, so it is obvious that it's disk-only, e.g.
virDomainGetDiskIOError, so we can have this name reserved for future.
But if you take the other way of letting this API to report IO errors
for any device, I am perfectly comfortable with disk-only implementation
for now.


 diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
 index 4ca7216..d778fdc 100644
 --- a/src/libvirt_public.syms
 +++ b/src/libvirt_public.syms
 @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 {
  virDomainSetNumaParameters;
  } LIBVIRT_0.9.8;
  
 +LIBVIRT_0.9.10 {
 +global:
 +virDomainGetIoError;
 +} LIBVIRT_0.9.9;
 +
  #  define new API here using predicted next version number 
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index e28840b..d89f45b 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -4750,6 +4750,7 @@ static virDriver remote_driver = {
  .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */
  

Re: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol

2012-01-25 Thread Michal Privoznik
On 25.01.2012 12:04, Michal Privoznik wrote:
 On 23.01.2012 14:30, Jiri Denemark wrote:
 We already provide ways to detect when a domain has been paused as a
 result of I/O error, but there was no way of getting the exact error or
 even the device that experienced it.  This new API may be used for both.
 ---
  include/libvirt/libvirt.h.in |   19 +++
  src/driver.h |6 
  src/libvirt.c|   53 
 ++
  src/libvirt_public.syms  |5 
  src/remote/remote_driver.c   |1 +
  src/remote/remote_protocol.x |   13 +-
  src/remote_protocol-structs  |9 +++
  7 files changed, 105 insertions(+), 1 deletions(-)

 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 958e5a6..2c3423a 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn,
 int interval,
 unsigned int count);
  
 +/**
 + * virDomainIoError:
 + *
 + * Disk I/O error.
 + */
 +typedef enum {
 +VIR_DOMAIN_IOERROR_NONE = 0, /* no error */
 +VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */
 +VIR_DOMAIN_IOERROR_UNSPEC   = 2, /* unspecified I/O error */
 
 Just cosmetic, since this is expected to be extended one day, I'd like
 to see _UNSPEC either at the beginning or at the end, but not in the
 middle. However, the latter is not possible as it's API change.
 But I can live with this thought.
 
 +
 +#ifdef VIR_ENUM_SENTINELS
 +VIR_DOMAIN_IOERROR_LAST
 +#endif
 +} virDomainIoError;
 +
 +int virDomainGetIoError(virDomainPtr dom,
 +const char *dev,
 +unsigned int flags);
 +
  #ifdef __cplusplus
  }
  #endif
 diff --git a/src/driver.h b/src/driver.h
 index 24636a4..1dd3760 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -794,6 +794,11 @@ typedef int
int *nparams,
unsigned int flags);
  
 +typedef int
 +(*virDrvDomainGetIoError)(virDomainPtr dom,
 +  const char *dev,
 +  unsigned int flags);
 +
  /**
   * _virDriver:
   *
 @@ -962,6 +967,7 @@ struct _virDriver {
  virDrvNodeSuspendForDuration nodeSuspendForDuration;
  virDrvDomainSetBlockIoTune domainSetBlockIoTune;
  virDrvDomainGetBlockIoTune domainGetBlockIoTune;
 +virDrvDomainGetIoError domainGetIoError;
  };
  
  typedef int
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 7b8adf7..99edca3 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -17882,3 +17882,56 @@ error:
  virDispatchError(dom-conn);
  return -1;
  }
 +
 +/**
 + * virDomainGetIoError:
 + * @dom: a domain object
 + * @dev: disk device to be inspected or NULL
 + * @flags: extra flags; not used yet, so callers should always pass 0
 + *
 + * The @disk parameter is either the device target (the dev attribute of
 + * target subelement), such as vda, or NULL, in which case all disks will 
 be
 + * inspected for errors. If only one disk experienced an I/O error, that 
 error
 + * will be returned. However, if there are more disks with I/O errors, this
 + * function will fail and return -2, requiring the caller to check every
 + * device explicitly.
 + *
 + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 
 if
 + * the function fails to do its job, the I/O error (virDomainIoError) 
 observed
 + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is 
 returned
 + * when there is no error on the disk.
 + */
 +int
 +virDomainGetIoError(virDomainPtr dom,
 +const char *dev,
 +unsigned int flags)
 
 Do we want to narrow this only for disks? Maybe some day hypervisors
 will report IO errors on other devices as well (haven't tried to find
 out if they do now days, though). However, if we restrict this only for
 disks, I'd change the name, so it is obvious that it's disk-only, e.g.
 virDomainGetDiskIOError, so we can have this name reserved for future.
 But if you take the other way of letting this API to report IO errors
 for any device, I am perfectly comfortable with disk-only implementation
 for now.
 

Oh, one more thing I thought is obvious but doesn't have to be for
others. In case you'll take the more generic way, let @dev be the alias
of the device which ought to be an unique string ID among the @dom.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/4] virsh: Implement domioerror command

2012-01-25 Thread Michal Privoznik
On 23.01.2012 14:30, Jiri Denemark wrote:
 ---
  tools/virsh.c   |  111 
 +++
  tools/virsh.pod |   11 +
  2 files changed, 122 insertions(+), 0 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index d635b56..92029fd 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -15809,6 +15809,116 @@ cleanup:
  }
  
  /*
 + * domioerror command
 + */
 +static const char *
 +vshDomainIOErrorToString(int error)
 +{
 +switch ((virDomainIoError) error) {

I like this typecast as once we add something to virDomainIoError but
forget to add it here, gcc will complain.

 +case VIR_DOMAIN_IOERROR_NONE:
 +return _(no error);
 +case VIR_DOMAIN_IOERROR_NO_SPACE:
 +return _(no space);
 +case VIR_DOMAIN_IOERROR_UNSPEC:
 +return _(unspecified error);
 +case VIR_DOMAIN_IOERROR_LAST:
 +;
 +}
 +
 +return _(unknown error);
 +}
 +

Overall, looking good, but see my comment to 1/4. You may need to change
this a bit then.

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] qemu: Implement virDomainIOError

2012-01-25 Thread Michal Privoznik
On 23.01.2012 14:30, Jiri Denemark wrote:
 ---
  src/qemu/qemu_conf.h |1 +
  src/qemu/qemu_driver.c   |   82 
 ++
  src/qemu/qemu_monitor.c  |   40 
  src/qemu/qemu_monitor.h  |1 +
  src/qemu/qemu_monitor_json.c |8 
  src/qemu/qemu_monitor_text.c |   15 
  6 files changed, 147 insertions(+), 0 deletions(-)
 

ACK, but see my comment to 1/4;

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 0/5] Introduce sVirt to LXC driver

2012-01-25 Thread Daniel P. Berrange
This is an update of

  https://www.redhat.com/archives/libvir-list/2012-January/msg00418.html

Changes since v1:

 - Pushed the first 2 patches which passed review
 - Update to include all Eric's suggested changes
 - Rebase to latest GIT master

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 4/5] Add support for sVirt in the LXC driver

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

For the sake of backwards compat, LXC guests are *not*
confined by default. This is because it is not practical
to dynamically relabel containers using large filesystem
trees. Applications can create confined containers though,
by giving suitable XML configs

* src/Makefile.am: Link libvirt_lxc to security drivers
* src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h,
  src/lxc/lxc_conf.c, src/lxc/lxc.conf,
  src/lxc/test_libvirtd_lxc.aug: Config file handling for
  security driver
* src/lxc/lxc_driver.c: Wire up security driver functions
* src/lxc/lxc_controller.c: Add a '--security' flag to
  specify which security driver to activate
* src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set
  the process label just before exec'ing init.
---
 src/Makefile.am   |   13 +++
 src/lxc/libvirtd_lxc.aug  |   15 +++-
 src/lxc/lxc.conf  |   18 
 src/lxc/lxc_conf.c|   62 --
 src/lxc/lxc_conf.h|8 ++-
 src/lxc/lxc_container.c   |9 ++-
 src/lxc/lxc_container.h   |2 +
 src/lxc/lxc_controller.c  |   28 +-
 src/lxc/lxc_driver.c  |  193 -
 src/lxc/test_libvirtd_lxc.aug |2 +
 10 files changed, 335 insertions(+), 15 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index a6f..7f1c80f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1503,7 +1503,14 @@ libvirt_lxc_SOURCES =
\
$(DOMAIN_CONF_SOURCES)  \
$(SECRET_CONF_SOURCES)  \
$(CPU_CONF_SOURCES) \
+   $(SECURITY_DRIVER_SOURCES)  \
$(NWFILTER_PARAM_CONF_SOURCES)
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES)
+endif
+if WITH_SECDRIVER_APPARMOR
+libvirt_lxc_SOURCES += $(SECURITY_DRIVER_APPARMOR_SOURCES)
+endif
 libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS)
 libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \
@@ -1513,6 +1520,9 @@ libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
 if WITH_DTRACE
 libvirt_lxc_LDADD += probes.o
 endif
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_LDADD += $(SELINUX_LIBS)
+endif
 libvirt_lxc_CFLAGS =   \
$(LIBPARTED_CFLAGS) \
$(NUMACTL_CFLAGS)   \
@@ -1525,6 +1535,9 @@ if HAVE_LIBBLKID
 libvirt_lxc_CFLAGS += $(BLKID_CFLAGS)
 libvirt_lxc_LDADD += $(BLKID_LIBS)
 endif
+if WITH_SECDRIVER_SELINUX
+libvirt_lxc_CFLAGS += $(SELINUX_CFLAGS)
+endif
 endif
 endif
 EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
diff --git a/src/lxc/libvirtd_lxc.aug b/src/lxc/libvirtd_lxc.aug
index 10f25e4..be6402c 100644
--- a/src/lxc/libvirtd_lxc.aug
+++ b/src/lxc/libvirtd_lxc.aug
@@ -7,13 +7,26 @@ module Libvirtd_lxc =
let value_sep   = del /[ \t]*=[ \t]*/   = 
let indent = del /[ \t]*/ 
 
+   let array_sep  = del /,[ \t\n]*/ , 
+   let array_start = del /\[[ \t\n]*/ [ 
+   let array_end = del /\]/ ]
+
+   let str_val = del /\/ \ . store /[^\]*/ . del /\/ \
let bool_val = store /0|1/
+   let int_val = store /[0-9]+/
+   let str_array_element = [ seq el . str_val ] . del /[ \t\n]*/ 
+   let str_array_val = counter el . array_start . ( str_array_element . ( 
array_sep . str_array_element ) * ) ? . array_end
 
+   let str_entry   (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry  (kw:string) = [ key kw . value_sep . bool_val ]
-
+   let int_entry   (kw:string) = [ key kw . value_sep . int_val ]
+   let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
 
(* Config entry grouped by function - same order as example config *)
let log_entry = bool_entry log_with_libvirtd
+ | str_entry security_driver
+ | bool_entry security_default_confined
+ | bool_entry security_require_confined
 
(* Each enty in the config is one of the following three ... *)
let entry = log_entry
diff --git a/src/lxc/lxc.conf b/src/lxc/lxc.conf
index 7a5066f..09dc95f 100644
--- a/src/lxc/lxc.conf
+++ b/src/lxc/lxc.conf
@@ -11,3 +11,21 @@
 # This is disabled by default, uncomment below to enable it.
 #
 # log_with_libvirtd = 1
+
+
+# The default security driver is SELinux. If SELinux is disabled
+# on the host, then the security driver will automatically disable
+# itself. If you wish to disable QEMU SELinux security driver while
+# leaving SELinux enabled for the host in general, then set this
+# to 'none' instead.
+#
+# security_driver = selinux
+
+# If set to non-zero, then the default security labeling
+# will make guests confined. If set to zero, then guests
+# will be unconfined by default. Defaults to 0.
+# security_default_confined = 1
+
+# If set to non-zero, then attempts to create unconfined
+# 

[libvirt] [PATCH v2 1/5] Revert changes to sec label parsing

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Revert parsing changes:

  commit 302fe95ffa1bc5f1c61c0beb31a1adfbc38c668e
  Author: Eric Blake ebl...@redhat.com
  Date:   Wed Jan 4 16:01:24 2012 -0700

seclabel: fix regression in libvirtd restart

  commit b43432931aef92325920953ff92beabfbe5224c8
  Author: Eric Blake ebl...@redhat.com
  Date:   Thu Dec 22 17:47:50 2011 -0700

seclabel: allow a seclabel override on a disk src

These two commits changed the sec label parsing code so that
the same code dealt with both the VM level sec label, and the
per device label. Unfortunately, as we add more options to the
VM level sec label, the logic required to use the same parsing
code for the per device label becomes unintelligible.

* src/conf/domain_conf.c: Remove support for parsing per
  device sec labels
---
 src/conf/domain_conf.c |  190 ---
 1 files changed, 49 insertions(+), 141 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e872396..6c67cd8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
 /*
  * domain_conf.c: domain XML processing
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2011 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -809,15 +809,6 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def)
 VIR_FREE(def-baselabel);
 }
 
-static void
-virSecurityLabelDefFree(virSecurityLabelDefPtr def)
-{
-if (!def)
-return;
-virSecurityLabelDefClear(def);
-VIR_FREE(def);
-}
-
 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 {
 int ii;
@@ -887,7 +878,6 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 
 VIR_FREE(def-serial);
 VIR_FREE(def-src);
-virSecurityLabelDefFree(def-seclabel);
 VIR_FREE(def-dst);
 VIR_FREE(def-driverName);
 VIR_FREE(def-driverType);
@@ -2508,33 +2498,31 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, 
virDomainDiskDefPtr def)
 return 0;
 }
 
-/* Parse the portion of a SecurityLabel that is common to both the
- * top-level seclabel and to a per-device override.
- * default_seclabel is NULL for top-level, or points to the top-level
- * when parsing an override.  */
 static int
-virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
-  xmlNodePtr node,
-  xmlXPathContextPtr ctxt,
-  virSecurityLabelDefPtr default_seclabel,
-  unsigned int flags)
+virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
+xmlXPathContextPtr ctxt,
+unsigned int flags)
 {
 char *p;
-xmlNodePtr save_ctxt = ctxt-node;
-int ret = -1;
-int type = default_seclabel ? default_seclabel-type : def-type;
 
-ctxt-node = node;
+if (virXPathNode(./seclabel, ctxt) == NULL)
+return 0;
 
-/* Can't use overrides if top-level doesn't allow relabeling.  */
-if (default_seclabel  default_seclabel-norelabel) {
-virDomainReportError(VIR_ERR_XML_ERROR, %s,
- _(label overrides require relabeling to be 
-   enabled at the domain level));
-goto cleanup;
+p = virXPathStringLimit(string(./seclabel/@type),
+VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+if (p == NULL) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ %s, _(missing security type));
+goto error;
 }
-
-p = virXPathStringLimit(string(./@relabel),
+def-type = virDomainSeclabelTypeFromString(p);
+VIR_FREE(p);
+if (def-type  0) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ %s, _(invalid security type));
+goto error;
+}
+p = virXPathStringLimit(string(./seclabel/@relabel),
 VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
 if (p != NULL) {
 if (STREQ(p, yes)) {
@@ -2545,77 +2533,38 @@ 
virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
 virDomainReportError(VIR_ERR_XML_ERROR,
  _(invalid security relabel value %s), p);
 VIR_FREE(p);
-goto cleanup;
+goto error;
 }
 VIR_FREE(p);
-if (!default_seclabel 
-type == VIR_DOMAIN_SECLABEL_DYNAMIC 
+if (def-type == VIR_DOMAIN_SECLABEL_DYNAMIC 
 def-norelabel) {
-virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(dynamic label type must use resource 
-   relabeling));
-goto cleanup;
+virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(dynamic label type must use resource 
relabeling));
+goto 

[libvirt] [PATCH v2 5/5] Set a security context on /dev and /dev/pts mounts

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

To allow the container to access /dev and /dev/pts when under
sVirt, set an explicit mount option. Also set a max size on
the /dev mount to prevent DOS on memory usage

* src/lxc/lxc_container.c: Set /dev mount context
* src/lxc/lxc_controller.c: Set /dev/pts mount context
---
 src/lxc/lxc_container.c  |   72 +++---
 src/lxc/lxc_controller.c |   32 ++--
 2 files changed, 84 insertions(+), 20 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b0493fd..c2ece84 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -36,6 +36,10 @@
 #include unistd.h
 #include mntent.h
 
+#if HAVE_SELINUX
+# include selinux/selinux.h
+#endif
+
 /* Yes, we want linux private one, for _syscall2() macro */
 #include linux/unistd.h
 
@@ -419,7 +423,6 @@ err:
 static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot)
 {
 const struct {
-bool onlyPivotRoot;
 bool needPrefix;
 const char *src;
 const char *dst;
@@ -433,16 +436,19 @@ static int lxcContainerMountBasicFS(const char 
*srcprefix, bool pivotRoot)
  * mount point in the main OS becomes readonly too which is not what
  * we want. Hence some things have two entries here.
  */
-{ true, false, devfs, /dev, tmpfs, mode=755, MS_NOSUID },
-{ false, false, proc, /proc, proc, NULL, 
MS_NOSUID|MS_NOEXEC|MS_NODEV },
-{ false, false, /proc/sys, /proc/sys, NULL, NULL, MS_BIND },
-{ false, false, /proc/sys, /proc/sys, NULL, NULL, 
MS_BIND|MS_REMOUNT|MS_RDONLY },
-{ false, true, /sys, /sys, NULL, NULL, MS_BIND },
-{ false, true, /sys, /sys, NULL, NULL, 
MS_BIND|MS_REMOUNT|MS_RDONLY },
-{ false, true, /selinux, /selinux, NULL, NULL, MS_BIND },
-{ false, true, /selinux, /selinux, NULL, NULL, 
MS_BIND|MS_REMOUNT|MS_RDONLY },
+{ false, proc, /proc, proc, NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV },
+{ false, /proc/sys, /proc/sys, NULL, NULL, MS_BIND },
+{ false, /proc/sys, /proc/sys, NULL, NULL, 
MS_BIND|MS_REMOUNT|MS_RDONLY },
+{ true, /sys, /sys, NULL, NULL, MS_BIND },
+{ true, /sys, /sys, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY },
+{ true, /selinux, /selinux, NULL, NULL, MS_BIND },
+{ true, /selinux, /selinux, NULL, NULL, 
MS_BIND|MS_REMOUNT|MS_RDONLY },
 };
 int i, rc = -1;
+char *opts = NULL;
+#if HAVE_SELINUX
+security_context_t con;
+#endif
 
 VIR_DEBUG(Mounting basic filesystems %s pivotRoot=%d, 
NULLSTR(srcprefix), pivotRoot);
 
@@ -450,10 +456,8 @@ static int lxcContainerMountBasicFS(const char *srcprefix, 
bool pivotRoot)
 char *src = NULL;
 const char *srcpath = NULL;
 
-VIR_DEBUG(Consider %s onlyPivotRoot=%d,
-  mnts[i].src, mnts[i].onlyPivotRoot);
-if (mnts[i].onlyPivotRoot  !pivotRoot)
-continue;
+VIR_DEBUG(Process %s - %s,
+  mnts[i].src, mnts[i].dst);
 
 if (virFileMakePath(mnts[i].dst)  0) {
 virReportSystemError(errno,
@@ -474,8 +478,10 @@ static int lxcContainerMountBasicFS(const char *srcprefix, 
bool pivotRoot)
 
 /* Skip if mount doesn't exist in source */
 if ((srcpath[0] == '/') 
-(access(srcpath, R_OK)  0))
+(access(srcpath, R_OK)  0)) {
+VIR_FREE(src);
 continue;
+}
 
 VIR_DEBUG(Mount %s on %s type=%s flags=%x, opts=%s,
   srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags, 
mnts[i].opts);
@@ -489,15 +495,47 @@ static int lxcContainerMountBasicFS(const char 
*srcprefix, bool pivotRoot)
 VIR_FREE(src);
 }
 
+if (pivotRoot) {
+#if HAVE_SELINUX
+if (getfilecon(/, con)  0 
+errno != ENOTSUP) {
+virReportSystemError(errno, %s,
+ _(Failed to query file context on /));
+goto cleanup;
+}
+#endif
+/*
+ * tmpfs is limited to 64kb, since we only have device nodes in there
+ * and don't want to DOS the entire OS RAM usage
+ */
+if (virAsprintf(opts, mode=755,size=65536%s%s%s,
+con ? ,context=\ : ,
+con ? (const char *)con : ,
+con ? \ : )  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+VIR_DEBUG(Mount devfs on /dev type=tmpfs flags=%x, opts=%s,
+  MS_NOSUID, opts);
+if (mount(devfs, /dev, tmpfs, MS_NOSUID, opts)  0) {
+virReportSystemError(errno,
+ _(Failed to mount %s on %s type %s),
+ devfs, /dev, tmpfs);
+goto cleanup;
+}
+}
+
 rc = 0;
 
 cleanup:
 VIR_DEBUG(rc=%d, rc);
+VIR_FREE(opts);
 return rc;
 }
 
 
-static int 

[libvirt] [PATCH v2 3/5] Add two new security label types

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Curently security labels can be of type 'dynamic' or 'static'.
If no security label is given, then 'dynamic' is assumed. The
current code takes advantage of this default, and avoids even
saving seclabel elements with type='dynamic' to disk. This
means if you temporarily change security driver, the guests
can all still start.

With the introduction of sVirt to LXC though, there needs to be
a new default of 'none' to allow unconfined LXC containers.

This patch introduces two new security label types

 - default:  the host configuration decides whether to run the
 guest with type 'none' or 'dynamic' at guest start
 - none: the guest will run unconfined by security policy

The 'none' label type will obviously be undesirable for some
deployments, so a new qemu.conf option allows a host admin to
mandate confined guests. It is also possible to turn off default
confinement

  security_default_confined = 1|0  (default == 1)
  security_require_confined = 1|0  (default == 0)

* src/conf/domain_conf.c, src/conf/domain_conf.h: Add new
  seclabel types
* src/security/security_manager.c, src/security/security_manager.h:
  Set default sec label types
* src/security/security_selinux.c: Handle 'none' seclabel type
* src/qemu/qemu.conf, src/qemu/qemu_conf.c, src/qemu/qemu_conf.h,
  src/qemu/libvirtd_qemu.aug: New security config options
* src/qemu/qemu_driver.c: Tell security driver about default
  config
---
 docs/formatdomain.html.in   |   24 +
 docs/schemas/domaincommon.rng   |5 +++
 po/POTFILES.in  |1 +
 src/conf/domain_conf.c  |   70 --
 src/conf/domain_conf.h  |2 +
 src/qemu/libvirtd_qemu.aug  |2 +
 src/qemu/qemu.conf  |8 
 src/qemu/qemu_conf.c|   11 ++
 src/qemu/qemu_conf.h|2 +
 src/qemu/qemu_driver.c  |7 +++-
 src/security/security_manager.c |   51 +---
 src/security/security_manager.h |8 -
 src/security/security_selinux.c |   32 ++
 tests/seclabeltest.c|2 +-
 14 files changed, 177 insertions(+), 48 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dfb010d..2d4f4cf 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null
 
 p
   The codeseclabel/code element allows control over the
-  operation of the security drivers. There are two basic
-  modes of operation, dynamic where libvirt automatically
-  generates a unique security label, or static where the
-  application/administrator chooses the labels. With dynamic
+  operation of the security drivers. There are three basic
+  modes of operation, 'dynamic' where libvirt automatically
+  generates a unique security label, 'static' where the
+  application/administrator chooses the labels, or 'none'
+  where confinement is disabled. With dynamic
   label generation, libvirt will always automatically
   relabel any resources associated with the virtual machine.
   With static label assignment, by default, the administrator
@@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null
   lt;seclabel type='static' model='selinux' relabel='yes'gt;
 lt;labelgt;system_u:system_r:svirt_t:s0:c392,c662lt;/labelgt;
   lt;/seclabelgt;
+
+  lt;seclabel type='none'/gt;
 /pre
 
 p
+  If no 'type' attribute is provided in the input XML, then
+  the security driver default setting will be used, which
+  may be either 'none' or 'static'. If a 'baselabel' is set
+  but no 'type' is set, then the type is presumed to be 'dynamic'
+/p
+
+p
   When viewing the XML for a running guest with automatic
   resource relabeling active, an additional XML element,
   codeimagelabel/code, will be included. This is an
@@ -3526,9 +3536,9 @@ qemu-kvm -net nic,model=? /dev/null
 /p
 dl
   dtcodetype/code/dt
-  ddEither codestatic/code or codedynamic/code to determine
-whether libvirt automatically generates a unique security label
-or not.
+  ddEither codestatic/code, codedynamic/code or codenone/code
+to determine whether libvirt automatically generates a unique security
+label or not.
   /dd
   dtcodemodel/code/dt
   ddA valid security model name, matching the currently
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4fa968d..6094643 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -116,6 +116,11 @@
 /optional
   /interleave
 /group
+group
+  attribute name='type'
+valuenone/value
+  /attribute
+/group
   /choice
 /element
   /define
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 

[libvirt] [PATCH v2 2/5] Re-add domain device seclabel parsing / formatting

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

This re-introduces parsing  formatting for per device seclabels.
There is a new virDomainDeviceSeclabelPtr struct and corresponding
APIs for parsing/formatting.
---
 src/conf/domain_conf.c |  132 ++--
 src/conf/domain_conf.h |   12 -
 2 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c67cd8..8eacc1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -809,6 +809,17 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def)
 VIR_FREE(def-baselabel);
 }
 
+
+static void
+virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
+{
+if (!def)
+return;
+VIR_FREE(def-label);
+VIR_FREE(def);
+}
+
+
 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 {
 int ii;
@@ -887,6 +898,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 virStorageEncryptionFree(def-encryption);
 virDomainDeviceInfoClear(def-info);
 
+virSecurityDeviceLabelDefFree(def-seclabel);
+
 for (i = 0 ; i  def-nhosts ; i++)
 virDomainDiskHostDefFree(def-hosts[i]);
 VIR_FREE(def-hosts);
@@ -2609,6 +2622,67 @@ error:
 return -1;
 }
 
+
+static int
+virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
+  virSecurityLabelDefPtr vmDef,
+  xmlXPathContextPtr ctxt)
+{
+char *p;
+
+*def = NULL;
+
+if (virXPathNode(./seclabel, ctxt) == NULL)
+return 0;
+
+/* Can't use overrides if top-level doesn't allow relabeling.  */
+if (vmDef  vmDef-norelabel) {
+virDomainReportError(VIR_ERR_XML_ERROR, %s,
+ _(label overrides require relabeling to be 
+   enabled at the domain level));
+return -1;
+}
+
+if (VIR_ALLOC(*def)  0) {
+virReportOOMError();
+return -1;
+}
+
+p = virXPathStringLimit(string(./seclabel/@relabel),
+VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+if (p != NULL) {
+if (STREQ(p, yes)) {
+(*def)-norelabel = false;
+} else if (STREQ(p, no)) {
+(*def)-norelabel = true;
+} else {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(invalid security relabel value %s), p);
+VIR_FREE(p);
+VIR_FREE(*def);
+return -1;
+}
+VIR_FREE(p);
+} else {
+(*def)-norelabel = false;
+}
+
+p = virXPathStringLimit(string(./seclabel/label[1]),
+VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+(*def)-label = p;
+
+if ((*def)-label  (*def)-norelabel) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(Cannot specify a label if relabelling is 
turned off));
+VIR_FREE((*def)-label);
+VIR_FREE(*def);
+return -1;
+}
+
+return 0;
+}
+
+
 /* Parse the XML definition for a lease
  */
 static virDomainLeaseDefPtr
@@ -2690,9 +2764,11 @@ virDomainDiskDefParseXML(virCapsPtr caps,
  xmlNodePtr node,
  xmlXPathContextPtr ctxt,
  virBitmapPtr bootMap,
+ virSecurityLabelDefPtr vmSeclabel,
  unsigned int flags)
 {
 virDomainDiskDefPtr def;
+xmlNodePtr sourceNode = NULL;
 xmlNodePtr cur, child;
 xmlNodePtr save_ctxt = ctxt-node;
 char *type = NULL;
@@ -2748,6 +2824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 if ((source == NULL  hosts == NULL) 
 (xmlStrEqual(cur-name, BAD_CAST source))) {
 
+sourceNode = cur;
+
 switch (def-type) {
 case VIR_DOMAIN_DISK_TYPE_FILE:
 source = virXMLPropString(cur, file);
@@ -2999,6 +3077,17 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 goto error;
 }
 
+/* If source is present, check for an optional seclabel override.  */
+if (sourceNode) {
+xmlNodePtr saved_node = ctxt-node;
+ctxt-node = sourceNode;
+if (virSecurityDeviceLabelDefParseXML(def-seclabel,
+  vmSeclabel,
+  ctxt)  0)
+goto error;
+ctxt-node = saved_node;
+}
+
 if (target == NULL) {
 virDomainReportError(VIR_ERR_NO_TARGET,
  source ? %s : NULL, source);
@@ -6346,7 +6435,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr 
caps,
 if (xmlStrEqual(node-name, BAD_CAST disk)) {
 dev-type = VIR_DOMAIN_DEVICE_DISK;
 if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node, ctxt,
-NULL, flags)))
+NULL, def-seclabel, 

Re: [libvirt] [PATCH v2 3/3] nwfilter: Rebuild filters only if new filter is different than current

2012-01-25 Thread Stefan Berger

On 01/23/2012 04:59 PM, Eric Blake wrote:

On 01/18/2012 09:20 AM, Stefan Berger wrote:

Compare two filter definitions for equality and only rebuild/instantiate
the new filter if the two filters are found to be different. This improves
performance during an update of a filter with no obvious change or the reloading
of filters during a 'kill -SIGHUP'

Unforuntately this more involved than a mere memcmp() on the structures.


2012

Big, but looks like a rather mechanical conversion of the various
protocols into a highly repetitive algorithm for comparisons.  I didn't
notice any major problems, so:

ACK with nits fixed.

After some consideration I decided not to push this patch but go a 
different route: have the new XML parsed, convert new and old internal 
representation of the filters to XML and compare those. This will be 
much less code.


   Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Add missing docs for viridian/ feature flag

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

---
 docs/formatdomain.html.in |4 
 docs/schemas/domaincommon.rng |5 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dfb010d..1d0211d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -866,6 +866,10 @@
   ddEnable use of Hardware Assisted Paging if available in
 the hardware.
   /dd
+  dtcodeviridian/code/dt
+  ddEnable Viridian hypervisor extensions for paravirtualizing
+guest operating systems
+  /dd
 /dl
 
 h3a name=elementsTimeTime keeping/a/h3
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4fa968d..2f655a9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2525,6 +2525,11 @@
   empty/
 /element
   /optional
+  optional
+element name=viridian
+  empty/
+/element
+  /optional
 /interleave
   /element
 /optional
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Add support for forcing a private network namespace for LXC guests

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

If no interface elements are included in an LXC guest XML
description, then the LXC guest will just see the host's
network interfaces. It is desirable to be able to hide the
host interfaces, without having to define any guest interfaces.

This patch introduces a new feature flag privnet/ to allow
forcing of a private network namespace for LXC. In the future
I also anticipate that we will add privuser/ to force a
private user ID namespace.

* src/conf/domain_conf.c, src/conf/domain_conf.h: Add support
  for privnet/ feature. Auto-set privnet if any interface
  devices are defined
* src/lxc/lxc_container.c: Honour request for private network
  namespace
---
 docs/formatdomain.html.in |7 +++
 docs/schemas/domaincommon.rng |5 +
 src/conf/domain_conf.c|3 ++-
 src/conf/domain_conf.h|1 +
 src/lxc/lxc_container.c   |   11 +++
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1d0211d..b7ad638 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -843,6 +843,7 @@
 lt;acpi/gt;
 lt;apic/gt;
 lt;hap/gt;
+lt;privnet/gt;
   lt;/featuresgt;
   .../pre
 
@@ -870,6 +871,12 @@
   ddEnable Viridian hypervisor extensions for paravirtualizing
 guest operating systems
   /dd
+  dtcodeprivnet/code/dt
+  ddAlways create a private network namespace. This is
+automatically set if any interface devices are defined.
+This feature is only relevant for container based
+virtualization drivers eg LXC.
+  /dd
 /dl
 
 h3a name=elementsTimeTime keeping/a/h3
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2f655a9..9f12a6d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2530,6 +2530,11 @@
   empty/
 /element
   /optional
+  optional
+element name=privnet
+  empty/
+/element
+  /optional
 /interleave
   /element
 /optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e872396..66d7f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -103,7 +103,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   apic,
   pae,
   hap,
-  viridian)
+  viridian,
+  privnet)
 
 VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST,
   destroy,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3b522a9..2131ff9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1221,6 +1221,7 @@ enum virDomainFeature {
 VIR_DOMAIN_FEATURE_PAE,
 VIR_DOMAIN_FEATURE_HAP,
 VIR_DOMAIN_FEATURE_VIRIDIAN,
+VIR_DOMAIN_FEATURE_PRIVNET,
 
 VIR_DOMAIN_FEATURE_LAST
 };
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index dcd65ef..0820a0e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -254,7 +254,8 @@ int lxcContainerWaitForContinue(int control)
  *
  * Returns 0 on success or nonzero in case of error
  */
-static int lxcContainerRenameAndEnableInterfaces(unsigned int nveths,
+static int lxcContainerRenameAndEnableInterfaces(bool privNet,
+ unsigned int nveths,
  char **veths)
 {
 int rc = 0;
@@ -282,7 +283,7 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned 
int nveths,
 }
 
 /* enable lo device only if there were other net devices */
-if (veths)
+if (veths || privNet)
 rc = virNetDevSetOnline(lo, true);
 
 error_out:
@@ -1277,7 +1278,8 @@ static int lxcContainerChild( void *data )
 VIR_DEBUG(Received container continue message);
 
 /* rename and enable interfaces */
-if (lxcContainerRenameAndEnableInterfaces(argv-nveths,
+if (lxcContainerRenameAndEnableInterfaces(vmDef-features  (1  
VIR_DOMAIN_FEATURE_PRIVNET),
+  argv-nveths,
   argv-veths)  0) {
 goto cleanup;
 }
@@ -1386,7 +1388,8 @@ int lxcContainerStart(virDomainDefPtr def,
 cflags |= CLONE_NEWUSER;
 }
 
-if (def-nets != NULL) {
+if (def-nets != NULL ||
+(def-features  (1  VIR_DOMAIN_FEATURE_PRIVNET))) {
 VIR_DEBUG(Enable network namespaces);
 cflags |= CLONE_NEWNET;
 }
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add missing docs for viridian/ feature flag

2012-01-25 Thread Eric Blake
On 01/25/2012 07:35 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 ---
  docs/formatdomain.html.in |4 
  docs/schemas/domaincommon.rng |5 +
  2 files changed, 9 insertions(+), 0 deletions(-)

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 0/5 0/1 0/1 V3] Add new public API virDomainGetPcpusUsage() and pcpuinfo command in virsh

2012-01-25 Thread Richard W.M. Jones

Is there a later version of this patch than V3?

This _needs_ to be accepted into libvirt 0.9.10 (ie. in 2 days) in
order for us to get this into RHEL 6.3.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Using Libvirt to change the bridge a virtual network card of a running vm is connected to

2012-01-25 Thread Hendrik Schwartke
I wrote a patch to change the mapping between a virtual bridge interface 
and the host bridge while the host is up. It's based on commit 
6fba577e505611e6c25c68e322942eab7754de7e. The host and the interface 
definition I used for testing are also attached.

I would be glad if the patch could be added to the repo.

Patch:

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4b60839..f791795 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -40,6 +40,7 @@
 #include qemu_cgroup.h
 #include locking/domain_lock.h
 #include network/bridge_driver.h
+#include util/virnetdevbridge.h

 #define VIR_FROM_THIS VIR_FROM_QEMU

@@ -1163,6 +1164,28 @@ static virDomainNetDefPtr 
qemuDomainFindNet(virDomainObjPtr vm,

 return NULL;
 }

+int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+  virDomainNetDefPtr newdev)
+{
+const char *oldbridge=olddev-data.bridge.brname;
+const char *newbridge=newdev-data.bridge.brname;
+VIR_DEBUG(Change bridge for interface %s: %s - %s, 
olddev-ifname, oldbridge, newbridge);

+if(virNetDevBridgeRemovePort(oldbridge, olddev-ifname)0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to remove port from bridge));
+return -1;
+}
+if(virNetDevBridgeAddPort(newbridge, newdev-ifname)0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to add port to bridge));
+return -1;
+}
+VIR_FREE(olddev-data.bridge.brname);
+olddev-data.bridge.brname=strdup(newbridge);
+return 0;
+}
+
+
 int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
  virDomainObjPtr vm,
  virDomainNetDefPtr dev,
@@ -1293,6 +1316,12 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
 return -1;
 }

+if(olddev-type==VIR_DOMAIN_NET_TYPE_BRIDGE
+  dev-type==VIR_DOMAIN_NET_TYPE_BRIDGE
+  STRNEQ(olddev-data.bridge.brname, dev-data.bridge.brname)) {
+qemuDomainChangeNetBridge(olddev, dev);
+}
+
 if (olddev-linkstate != dev-linkstate) {
 if ((ret = qemuDomainChangeNetLinkState(driver, vm, olddev, 
dev-linkstate))  0)

 return ret;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0310361..1e1f75c 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -80,6 +80,8 @@ int qemuDomainChangeNetLinkState(struct qemud_driver 
*driver,

  virDomainObjPtr vm,
  virDomainNetDefPtr dev,
  int linkstate);
+int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+  virDomainNetDefPtr newdev);
 int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
   virDomainObjPtr vm,
   virDomainDeviceDefPtr dev);


Host definition:
domain type='kvm'
nametest/name
memory32768/memory
currentMemory32768/currentMemory
vcpu1/vcpu
os
type arch='x86_64' machine='pc-0.12'hvm/type
boot dev='cdrom'/
bootmenu enable='no'/
/os
features
acpi/
apic/
pae/
/features
clock offset='utc'/
on_poweroffdestroy/on_poweroff
on_rebootrestart/on_reboot
on_crashrestart/on_crash
devices
emulator/usr/bin/kvm/emulator
controller type='ide' index='0'
address type='pci' domain='0x' bus='0x00' slot='0x01' function='0x1'/
/controller
interface type='bridge'
mac address='52:54:00:ab:cd:02'/
source bridge='br0'/
target dev='testif'/
link state='up'/
address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/
/interface
serial type='pty'
target port='0'/
/serial
console type='pty'
target type='serial' port='0'/
/console
input type='tablet' bus='usb'/
input type='mouse' bus='ps2'/
graphics type='vnc' port='-1' autoport='yes'/
video
model type='cirrus' vram='9216' heads='1'/
address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/
/video
memballoon model='virtio'
address type='pci' domain='0x' bus='0x00' slot='0x06' function='0x0'/
/memballoon
/devices
/domain


Interface definition:
interface type=bridge
mac address=52:54:00:ab:cd:02/
source bridge=br1/
target dev=testif/
link state=up/
alias name=net0/
address type=pci domain=0x bus=0x00 slot=0x03 function=0x0/
/interface



Best regards
Hendrik Schwartke

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] lxc: export container=lxc-libvirt for systemd

2012-01-25 Thread Serge Hallyn
Quoting Daniel P. Berrange (berra...@redhat.com):
 On Tue, Jan 24, 2012 at 11:53:38AM -0700, Eric Blake wrote:
  Systemd detects containers based on whether they have
  an environment variable starting with 'container=lxc';
  using a longer name fits the expectations, while also
  allowing detection of who created the container.
  
  Requested by Lennart Poettering, in response to
  https://bugs.freedesktop.org/show_bug.cgi?id=45175
  
  * src/lxc/lxc_container.c (lxcContainerBuildInitCmd): Add another
  env-var.
  ---
   src/lxc/lxc_container.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
  
  diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
  index c1ec9c4..70f6908 100644
  --- a/src/lxc/lxc_container.c
  +++ b/src/lxc/lxc_container.c
  @@ -1,5 +1,5 @@
   /*
  - * Copyright (C) 2008-2011 Red Hat, Inc.
  + * Copyright (C) 2008-2012 Red Hat, Inc.
* Copyright (C) 2008 IBM Corp.
*
* lxc_container.c: file description
  @@ -124,6 +124,7 @@ static virCommandPtr 
  lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
  
   virCommandAddEnvString(cmd, PATH=/bin:/sbin);
   virCommandAddEnvString(cmd, TERM=linux);
  +virCommandAddEnvString(cmd, container=lxc-libvirt);
   virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr);
   virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name);
   if (vmDef-os.cmdline)
 
 ACK, I had actually offered to do this before, but at the time
 it was though having LIBVIRT_LXC_UUID was fine.

And it still is...  right?  container= is just deemed prettier?

(just curious)

 Oh well.

-serge

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] schema: Relax schema for domain name

2012-01-25 Thread Peter Krempa

On 01/23/2012 07:09 PM, Daniel P. Berrange wrote:

On Mon, Jan 23, 2012 at 06:53:17PM +0100, Peter Krempa wrote:

The domain schema enforced restrictions on the domain name string that
the code doesn't. This patch relaxes the check, leaving the restrictions
on the driver or hypervisor.
---
And maybe we should consider adding some restrictions on the qemu driver, as 
the daemon
is competely fine with creating a domain with the name 
../../../../../../../test that
has its configuration stored in /test.xml then.


I don't think we should remove the pattern entirely. If we want a more
general pattern though, we could do an 'allow all', and blacklist
just '/' and perhaps a few other characters.


Well, slash is one of those symbols, that some hypervisors happily take 
as a valid domain name without screwing up their config files. I think 
we should blacklist only the newline and let the hypervisor decide what 
they accept and what not.


Peter



I think we should also fix the drivers to check this, since once we
have stricter access control support in libvirt, the kind of issue
you describe with QEMU will be classed as a CVE security exploit.


Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add support for forcing a private network namespace for LXC guests

2012-01-25 Thread Eric Blake
On 01/25/2012 07:35 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 If no interface elements are included in an LXC guest XML
 description, then the LXC guest will just see the host's
 network interfaces. It is desirable to be able to hide the
 host interfaces, without having to define any guest interfaces.
 
 This patch introduces a new feature flag privnet/ to allow
 forcing of a private network namespace for LXC. In the future
 I also anticipate that we will add privuser/ to force a
 private user ID namespace.
 
 * src/conf/domain_conf.c, src/conf/domain_conf.h: Add support
   for privnet/ feature. Auto-set privnet if any interface
   devices are defined
 * src/lxc/lxc_container.c: Honour request for private network
   namespace
 ---

 @@ -870,6 +871,12 @@
ddEnable Viridian hypervisor extensions for paravirtualizing
  guest operating systems
/dd
 +  dtcodeprivnet/code/dt
 +  ddAlways create a private network namespace. This is
 +automatically set if any interface devices are defined.
 +This feature is only relevant for container based
 +virtualization drivers eg LXC.

s/drivers eg/drivers, such as/

 +++ b/src/lxc/lxc_container.c
 @@ -254,7 +254,8 @@ int lxcContainerWaitForContinue(int control)
   *
   * Returns 0 on success or nonzero in case of error
   */
 -static int lxcContainerRenameAndEnableInterfaces(unsigned int nveths,
 +static int lxcContainerRenameAndEnableInterfaces(bool privNet,
 + unsigned int nveths,
   char **veths)
  {
  int rc = 0;
 @@ -282,7 +283,7 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned 
 int nveths,
  }
  
  /* enable lo device only if there were other net devices */
 -if (veths)
 +if (veths || privNet)
  rc = virNetDevSetOnline(lo, true);
  
  error_out:
 @@ -1277,7 +1278,8 @@ static int lxcContainerChild( void *data )
  VIR_DEBUG(Received container continue message);
  
  /* rename and enable interfaces */
 -if (lxcContainerRenameAndEnableInterfaces(argv-nveths,
 +if (lxcContainerRenameAndEnableInterfaces(vmDef-features  (1  
 VIR_DOMAIN_FEATURE_PRIVNET),

I'm still a bit leery of relying on C99 conversion to bool; I would
write this as:

!!(vm-def-features  (1  VIR_DOMAIN_FEATURE_PRIVNET))

or similar.  But this wouldn't be the first time we rely on the compiler
obeying the spec without us having to add extra syntax.

 @@ -1386,7 +1388,8 @@ int lxcContainerStart(virDomainDefPtr def,
  cflags |= CLONE_NEWUSER;
  }
  
 -if (def-nets != NULL) {
 +if (def-nets != NULL ||
 +(def-features  (1  VIR_DOMAIN_FEATURE_PRIVNET))) {

On the other hand, this use is fine (that is, passing int to a bool
parameter is risky, using int in || is not).

ACK, whether or not you change the syntax of the call to
lxcContainerRenameAndEnableInterfaces.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] [PATCHv2] schema: Relax schema for domain name

2012-01-25 Thread Peter Krempa
The domain schema enforced restrictions on the domain name string that
the code doesn't. This patch relaxes the check, leaving the restrictions
on the driver or hypervisor. The only invalid character is a newline.
---
 docs/schemas/domaincommon.rng |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4fa968d..ecf3484 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3030,7 +3030,7 @@
   /define
   define name=domainName
 data type=string
-  param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param
+  param name=pattern[^\n]+/param
 /data
   /define
   define name=diskSerial
-- 
1.7.3.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] lxc: export container=lxc-libvirt for systemd

2012-01-25 Thread Eric Blake
On 01/25/2012 08:18 AM, Serge Hallyn wrote:
 Quoting Daniel P. Berrange (berra...@redhat.com):
 On Tue, Jan 24, 2012 at 11:53:38AM -0700, Eric Blake wrote:
 Systemd detects containers based on whether they have
 an environment variable starting with 'container=lxc';
 using a longer name fits the expectations, while also
 allowing detection of who created the container.

 Requested by Lennart Poettering, in response to
 https://bugs.freedesktop.org/show_bug.cgi?id=45175


 ACK, I had actually offered to do this before, but at the time
 it was though having LIBVIRT_LXC_UUID was fine.
 
 And it still is...  right?  container= is just deemed prettier?

According to the systemd bug above, yes.  Also, from my IRC conversation
with Lennart, I came away with the idea that other containers are also
setting $container, so having ALL containers set the same environment
variable (and realizing that other containers obviously won't be using
LIBVIRT_LXC_*) makes it easier for systemd to recognize all containers
from just a single variable name, rather than having to hand-maintain a
growing list as new containers are created.

At any rate, I've pushed the patch now.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] [PATCHv2] schema: Relax schema for domain name

2012-01-25 Thread Eric Blake
On 01/25/2012 08:49 AM, Peter Krempa wrote:
 The domain schema enforced restrictions on the domain name string that
 the code doesn't. This patch relaxes the check, leaving the restrictions
 on the driver or hypervisor. The only invalid character is a newline.
 ---
  docs/schemas/domaincommon.rng |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 4fa968d..ecf3484 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -3030,7 +3030,7 @@
/define
define name=domainName
  data type=string
 -  param name=pattern[A-Za-z0-9_\.\+\-amp;:/]+/param
 +  param name=pattern[^\n]+/param

Also, an empty name is forbidden both pre- and post-patch.  I agree that
any further restrictions should come from the drivers, so I'm okay giving:

ACK

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] Replace hashing algorithm with murmurhash

2012-01-25 Thread Daniel P. Berrange
On Wed, Jan 18, 2012 at 11:34:16AM -0700, Eric Blake wrote:
 On 01/18/2012 09:23 AM, Daniel P. Berrange wrote:
  @@ -1636,9 +1637,10 @@ cleanup:
   }
   
   
  -static unsigned long virCgroupPidCode(const void *name)
  +static int32_t virCgroupPidCode(const void *name, int32_t seed)
   {
  -return (unsigned long)name;
  +unsigned long pid = (unsigned long)name;
  +return virHashCodeGen(pid, sizeof(pid), seed);
 
 I'm not sure if it matters, but you could shorten these two lines to
 just one:
 
 return virHashCodeGen(name, sizeof(unsigned long), seed);

I actually preferred the slightly more verbose style, to make
it clearer that we're actually passing in the PID as an int,
cast to a pointer, instead of an int pointer.

  @@ -101,11 +95,10 @@ static void virHashStrFree(void *name)
   VIR_FREE(name);
   }
   
  -
   static unsigned long
   virHashComputeKey(virHashTablePtr table, const void *name)
   {
  -unsigned long value = table-keyCode(name);
  +uint32_t value = table-keyCode(name, table-seed);
   return (value % table-size);
 
 If, on a 64-bit machine, table-size ever grows larger than uint32_t,
 then we've lost some hashing abilities compared to what the old unsigned
 long hash gave us.  Conversely, if table-size ever grows that large,
 we're burning an awful lot of memory in the first place, and a hash
 collision at that point is the least of our worries :)  I think this is
 okay, although you may want to consider changing virHashComputeKey to
 return uint32_t.

I changed the signature here to actually be  'size_t', likewise
for the table-size variable itself.


  +++ b/src/util/hash.h
  @@ -55,12 +55,15 @@ typedef int (*virHashSearcher) (const void *payload, 
  const void *name,
   /**
* virHashKeyCode:
* @name: the hash key
  + * @seed: random seed
*
  - * Compute the hash code corresponding to the key @name
  + * Compute the hash code corresponding to the key @name, using
  + * @seed to perturb the hashing algorithm
*
* Returns the hash code
*/
  -typedef unsigned long (*virHashKeyCode)(const void *name);
  +typedef int32_t (*virHashKeyCode)(const void *name,
  +  int32_t seed);
 
 Umm, you used uint32_t in virHashComputeKey, but int32_t here.  While I
 don't _think_ signed values will bite us due to sign-extension in 64-bit
 operations introducing a bias, I'd rather make this signature:
 
 typedef uint32_t (*virHashKeyCode)(const void *name, uint32_t seed);
 
 and adjust the callers to comply with unsigned everywhere.

Yeah, that was a mistake - it should have been uint32.

 That is, I propose that we ditch virRandom, and instead implement a new
 function:
 
 /* Return an evenly distributed random number between [0,2^nbits), where
 nbits must be in the range (0,64]. */
 uint64_t virRandomBits(int nbits)
 {
 int bits_per_iter = count_one_bits(RAND_MAX);
 uint64_t ret = 0;
 int32_t bits;
 /* This algorithm requires that RAND_MAX == 2^n-1 for some n;
gnulib's random_r meets this property. */
 verify(((RAND_MAX + 1U)  RAND_MAX) == 0);
 
 virMutexLock(randomLock);
 
 while (nbits  bits_per_iter) {
 random_r(randomData, bits);
 ret = (ret  bits_per_iter) | (bits  RAND_MAX);
 nbits -= bits_per_iter;
 }
 
 random_r(randomData, bits);
 ret = (ret  nbits) | (bits  ((1  nbits) - 1));
 
 virMutexUnlock(randomLock);
 return ret;
 }
 
 where existing calls to virRandom(256) will now be virRandomBits(8),
 virRandom(1024*1024*1024) will now be virRandomBits(30), and your code
 would use uint32_t seed = virRandom(32).

Yep, I've done that.

  +#include virhashcode.h
  +
  +static uint32_t rotl(uint32_t x, int8_t r)
  +{
  +return (x  r) | (x  (32 - r));
  +}
 
 Gnulib provides the bitrotate module, which provides bitrotate.h and
 an implementation of this function that is guaranteed to be as efficient
 as possible when compiled by gcc (possibly by using compiler-builtins to
 access raw assembly on architectures that have builtin rotate
 instructions).  I'd suggest using that rather than rolling your own.

Good point.

 
  +
  +/* slower than original but is endian neutral and handles platforms that
  + * do only aligned reads */
  +__attribute__((always_inline))
  +static uint32_t getblock(const uint8_t *p, int i)
  +{
  +uint32_t r;
  +size_t size = sizeof(uint32_t);
  +
  +memcpy(r, p[i * size], size);
  +
  +return le32toh(r);
 
 endian.h, and thus le32toh(), is not yet standardized (although POSIX
 will be adding it in the future), nor is it currently provided by
 gnulib.  We'd have to get that fixed first.

The le32toh call was only here because the code I copied wanted to be
endian neutral. I don't think libvirt really cares if its hash codes
are endian neutral, so I trivially just removed the le32toh call and
avoid the problem of endian.h


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ 

[libvirt] [PATCH 2/4] Convert various virHash functions to use size_t / uint32

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

In preparation for conversion over to use the Murmurhash3
algorithm, convert various virHash APIs to use size_t or
uint32 for their return values/parameters, instead of the
variable size 'unsigned long' or 'int' types
---
 src/util/cgroup.c |4 +-
 src/util/hash.c   |   56 +++-
 src/util/hash.h   |   14 ++--
 3 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 25f2691..a270965 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -1636,9 +1636,9 @@ cleanup:
 }
 
 
-static unsigned long virCgroupPidCode(const void *name)
+static uint32_t virCgroupPidCode(const void *name)
 {
-return (unsigned long)name;
+return (uint32_t)(int64_t)name;
 }
 static bool virCgroupPidEqual(const void *namea, const void *nameb)
 {
diff --git a/src/util/hash.c b/src/util/hash.c
index ff86f16..20d3a12 100644
--- a/src/util/hash.c
+++ b/src/util/hash.c
@@ -57,8 +57,8 @@ struct _virHashEntry {
  */
 struct _virHashTable {
 virHashEntryPtr *table;
-int size;
-int nbElems;
+size_t size;
+size_t nbElems;
 /* True iff we are iterating over hash entries. */
 bool iterating;
 /* Pointer to the current entry during iteration. */
@@ -70,17 +70,17 @@ struct _virHashTable {
 virHashKeyFree keyFree;
 };
 
-static unsigned long virHashStrCode(const void *name)
+static uint32_t virHashStrCode(const void *name)
 {
 const char *str = name;
-unsigned long value = 0L;
+uint32_t value = 0L;
 char ch;
 
 if (str != NULL) {
 value += 30 * (*str);
 while ((ch = *str++) != 0) {
 value =
-value ^ ((value  5) + (value  3) + (unsigned long) ch);
+value ^ ((value  5) + (value  3) + (uint32_t) ch);
 }
 }
 return value;
@@ -102,10 +102,10 @@ static void virHashStrFree(void *name)
 }
 
 
-static unsigned long
+static size_t
 virHashComputeKey(virHashTablePtr table, const void *name)
 {
-unsigned long value = table-keyCode(name);
+uint32_t value = table-keyCode(name);
 return (value % table-size);
 }
 
@@ -122,7 +122,7 @@ virHashComputeKey(virHashTablePtr table, const void *name)
  *
  * Returns the newly created object, or NULL if an error occurred.
  */
-virHashTablePtr virHashCreateFull(int size,
+virHashTablePtr virHashCreateFull(ssize_t size,
   virHashDataFree dataFree,
   virHashKeyCode keyCode,
   virHashKeyEqual keyEqual,
@@ -166,7 +166,7 @@ virHashTablePtr virHashCreateFull(int size,
  *
  * Returns the newly created object, or NULL if an error occurred.
  */
-virHashTablePtr virHashCreate(int size, virHashDataFree dataFree)
+virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree)
 {
 return virHashCreateFull(size,
  dataFree,
@@ -186,13 +186,13 @@ virHashTablePtr virHashCreate(int size, virHashDataFree 
dataFree)
  * Returns 0 in case of success, -1 in case of failure
  */
 static int
-virHashGrow(virHashTablePtr table, int size)
+virHashGrow(virHashTablePtr table, size_t size)
 {
-int oldsize, i;
+size_t oldsize, i;
 virHashEntryPtr *oldtable;
 
 #ifdef DEBUG_GROW
-unsigned long nbElem = 0;
+size_t nbElem = 0;
 #endif
 
 if (table == NULL)
@@ -218,7 +218,7 @@ virHashGrow(virHashTablePtr table, int size)
 virHashEntryPtr iter = oldtable[i];
 while (iter) {
 virHashEntryPtr next = iter-next;
-unsigned long key = virHashComputeKey(table, iter-name);
+size_t key = virHashComputeKey(table, iter-name);
 
 iter-next = table-table[key];
 table-table[key] = iter;
@@ -250,7 +250,7 @@ virHashGrow(virHashTablePtr table, int size)
 void
 virHashFree(virHashTablePtr table)
 {
-int i;
+size_t i;
 
 if (table == NULL)
 return;
@@ -278,7 +278,7 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void 
*name,
 void *userdata,
 bool is_update)
 {
-unsigned long key, len = 0;
+size_t key, len = 0;
 virHashEntryPtr entry;
 char *new_name;
 
@@ -372,7 +372,7 @@ virHashUpdateEntry(virHashTablePtr table, const void *name,
 void *
 virHashLookup(virHashTablePtr table, const void *name)
 {
-unsigned long key;
+size_t key;
 virHashEntryPtr entry;
 
 if (!table || !name)
@@ -419,7 +419,7 @@ void *virHashSteal(virHashTablePtr table, const void *name)
  * Returns the number of elements in the hash table or
  * -1 in case of error
  */
-int
+ssize_t
 virHashSize(virHashTablePtr table)
 {
 if (table == NULL)
@@ -436,7 +436,7 @@ virHashSize(virHashTablePtr table)
  * Returns the number of keys in the hash table or
  * -1 in case of error
  */
-int
+ssize_t
 virHashTableSize(virHashTablePtr table)
 {
 if (table == NULL)
@@ 

[libvirt] [PATCH v2 0/4] Update the hash table algorithm

2012-01-25 Thread Daniel P. Berrange
This is a followup to

  https://www.redhat.com/archives/libvir-list/2012-January/msg00734.html

Changes in v2:

 - Replace virRandom with virRandomBits as per Eric's suggested impl
 - Move virRandom* functions to virrandom.{c,h}
 - Move virHash impl to virhash.{c,h}
 - Remove use of le32toh() function
 - Convert several virHash functions to use size_t / uint32 instead
   of int/unsigned long
 - Split into 4 patches for easier review/bisect

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/4] Introduce new API for generating random numbers

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The old virRandom() API was not generating good random numbers.
Replace it with a new API virRandomBits which instead of being
told the upper limit, gets told the number of bits of randomness
required.

* src/util/virrandom.c, src/util/virrandom.h: Add virRandomBits,
  and move virRandomInitialize
* src/util/util.h, src/util/util.c: Delete virRandom and
  virRandomInitialize
* src/libvirt.c, src/security/security_selinux.c,
  src/test/test_driver.c, src/util/iohelper.c: Update for
  changes from virRandom to virRandomBits
* src/storage/storage_backend_iscsi.c: Remove bogus call
  to virRandomInitialize  convert to virRandomBits
---
 src/Makefile.am |1 +
 src/libvirt.c   |2 +-
 src/libvirt_private.syms|7 ++-
 src/security/security_selinux.c |5 +-
 src/storage/storage_backend_iscsi.c |   12 +
 src/test/test_driver.c  |3 +-
 src/util/iohelper.c |1 +
 src/util/util.c |   37 ++
 src/util/util.h |3 -
 src/util/uuid.c |3 +-
 src/util/virrandom.c|   73 +++
 src/util/virrandom.h|   30 ++
 12 files changed, 125 insertions(+), 52 deletions(-)
 create mode 100644 src/util/virrandom.c
 create mode 100644 src/util/virrandom.h

diff --git a/src/Makefile.am b/src/Makefile.am
index a6f..4629700 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -99,6 +99,7 @@ UTIL_SOURCES =
\
util/virnetdevtap.h util/virnetdevtap.c \
util/virnetdevveth.h util/virnetdevveth.c \
util/virnetdevvportprofile.h util/virnetdevvportprofile.c \
+   util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
util/virtime.h util/virtime.c
 
diff --git a/src/libvirt.c b/src/libvirt.c
index 722a2e2..68eec2e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -36,7 +36,6 @@
 #include driver.h
 
 #include uuid.h
-#include util.h
 #include memory.h
 #include configmake.h
 #include intprops.h
@@ -44,6 +43,7 @@
 #include rpc/virnettlscontext.h
 #include command.h
 #include virnodesuspend.h
+#include virrandom.h
 
 #ifndef WITH_DRIVER_MODULES
 # ifdef WITH_TEST
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 924ec16..915a43f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1126,8 +1126,6 @@ virParseMacAddr;
 virParseNumber;
 virParseVersionString;
 virPipeReadUntilEOF;
-virRandom;
-virRandomInitialize;
 virSetBlocking;
 virSetCloseExec;
 virSetInherit;
@@ -1369,6 +1367,11 @@ virPidFileDelete;
 virPidFileDeletePath;
 
 
+# virrandom.h
+virRandomBits;
+virRandomInitialize;
+
+
 # virsocketaddr.h
 virSocketAddrBroadcast;
 virSocketAddrBroadcastByPrefix;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c2dceca..cb41a17 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -32,6 +32,7 @@
 #include hostusb.h
 #include storage_file.h
 #include virfile.h
+#include virrandom.h
 
 #define VIR_FROM_THIS VIR_FROM_SECURITY
 
@@ -216,8 +217,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED,
 }
 } else {
 do {
-c1 = virRandom(1024);
-c2 = virRandom(1024);
+c1 = virRandomBits(10);
+c2 = virRandomBits(10);
 
 if ( c1 == c2 ) {
 if (virAsprintf(mcs, s0:c%d, c1)  0) {
diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index 354f99b..6ebc6e1 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -42,6 +42,7 @@
 #include logging.h
 #include virfile.h
 #include command.h
+#include virrandom.h
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -283,15 +284,8 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
 initiatoriqn, NULL
 };
 
-if (virRandomInitialize(time(NULL) ^ getpid()) == -1) {
-virStorageReportError(VIR_ERR_INTERNAL_ERROR, %s,
-  _(Failed to initialize random generator 
-when creating iscsi interface));
-goto out;
-}
-
-snprintf(temp_ifacename, sizeof(temp_ifacename), libvirt-iface-%08x,
- virRandom(1024 * 1024 * 1024));
+snprintf(temp_ifacename, sizeof(temp_ifacename), libvirt-iface-%08llx,
+ (unsigned long long)virRandomBits(30));
 
 VIR_DEBUG(Attempting to create interface '%s' with IQN '%s',
   temp_ifacename[0], initiatoriqn);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 55b889b..bf6b148 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -51,6 +51,7 @@
 #include 

[libvirt] [PATCH 4/4] Replace hashing algorithm with murmurhash

2012-01-25 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Recent discussions have illustrated the potential for DOS attacks
with the hash table implementations used by most languages and
libraries.

   https://lwn.net/Articles/474912/

libvirt has an internal hash table impl, and uses hash tables for
a variety of purposes. The hash key generation code is pretty
simple and thus not strongly collision resistant.

This patch replaces the current libvirt hash key generator with
the (public domain) Murmurhash3 code. In addition every hash
table now gets a random seed value which is used to perturb the
hashing code. This should make it impossible to mount any
practical attack against libvirt hashing code.

* bootstrap.conf: Import bitrotate module
* src/Makefile.am: Add virhashcode.[ch]
* src/util/util.c: Make virRandom() return a fixed 32 bit
  integer value.
* src/util/hash.c, src/util/hash.h, src/util/cgroup.c: Replace
  hash code generation with a call to virHashCodeGen()
* src/util/virhashcode.h, src/util/virhashcode.c: Add a new
  virHashCodeGen() API using the Murmurhash3 algorithm.
---
 bootstrap.conf |1 +
 src/Makefile.am|1 +
 src/util/cgroup.c  |6 ++-
 src/util/virhash.c |   21 +++--
 src/util/virhash.h |7 ++-
 src/util/virhashcode.c |  121 
 src/util/virhashcode.h |   35 ++
 7 files changed, 174 insertions(+), 18 deletions(-)
 create mode 100644 src/util/virhashcode.c
 create mode 100644 src/util/virhashcode.h

diff --git a/bootstrap.conf b/bootstrap.conf
index 04c7645..03da267 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -23,6 +23,7 @@ accept
 areadlink
 base64
 bind
+bitrotate
 byteswap
 c-ctype
 c-strcase
diff --git a/src/Makefile.am b/src/Makefile.am
index eacc741..22fd58e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -90,6 +90,7 @@ UTIL_SOURCES =
\
util/xml.c util/xml.h   \
util/virterror.c util/virterror_internal.h  \
util/virhash.c util/virhash.h   \
+   util/virhashcode.c util/virhashcode.h   \
util/virkeycode.c util/virkeycode.h \
util/virkeymaps.h   \
util/virnetdev.h util/virnetdev.c   \
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 7d7570a..828ad66 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -33,6 +33,7 @@
 #include logging.h
 #include virfile.h
 #include virhash.h
+#include virhashcode.h
 
 #define CGROUP_MAX_VAL 512
 
@@ -1636,9 +1637,10 @@ cleanup:
 }
 
 
-static uint32_t virCgroupPidCode(const void *name)
+static uint32_t virCgroupPidCode(const void *name, uint32_t seed)
 {
-return (uint32_t)(int64_t)name;
+unsigned long pid = (unsigned long)name;
+return virHashCodeGen(pid, sizeof(pid), seed);
 }
 static bool virCgroupPidEqual(const void *namea, const void *nameb)
 {
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 7294c7e..4ee1a1c 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -28,6 +28,8 @@
 #include virhash.h
 #include memory.h
 #include logging.h
+#include virhashcode.h
+#include virrandom.h
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -57,6 +59,7 @@ struct _virHashEntry {
  */
 struct _virHashTable {
 virHashEntryPtr *table;
+uint32_t seed;
 size_t size;
 size_t nbElems;
 /* True iff we are iterating over hash entries. */
@@ -70,20 +73,9 @@ struct _virHashTable {
 virHashKeyFree keyFree;
 };
 
-static uint32_t virHashStrCode(const void *name)
+static uint32_t virHashStrCode(const void *name, uint32_t seed)
 {
-const char *str = name;
-uint32_t value = 0L;
-char ch;
-
-if (str != NULL) {
-value += 30 * (*str);
-while ((ch = *str++) != 0) {
-value =
-value ^ ((value  5) + (value  3) + (uint32_t) ch);
-}
-}
-return value;
+return virHashCodeGen(name, strlen(name), seed);
 }
 
 static bool virHashStrEqual(const void *namea, const void *nameb)
@@ -105,7 +97,7 @@ static void virHashStrFree(void *name)
 static size_t
 virHashComputeKey(virHashTablePtr table, const void *name)
 {
-uint32_t value = table-keyCode(name);
+uint32_t value = table-keyCode(name, table-seed);
 return (value % table-size);
 }
 
@@ -139,6 +131,7 @@ virHashTablePtr virHashCreateFull(ssize_t size,
 return NULL;
 }
 
+table-seed = virRandomBits(32);
 table-size = size;
 table-nbElems = 0;
 table-dataFree = dataFree;
diff --git a/src/util/virhash.h b/src/util/virhash.h
index 2420045..7360676 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -55,12 +55,15 @@ typedef int (*virHashSearcher) (const void *payload, const 
void *name,
 /**
  * virHashKeyCode:
  * @name: the hash key
+ * @seed: random seed
  *
- * Compute the hash code 

Re: [libvirt] [PATCH] Replace hashing algorithm with murmurhash

2012-01-25 Thread Eric Blake
On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 -static unsigned long virCgroupPidCode(const void *name)
 +static int32_t virCgroupPidCode(const void *name, int32_t seed)
  {
 -return (unsigned long)name;
 +unsigned long pid = (unsigned long)name;
 +return virHashCodeGen(pid, sizeof(pid), seed);

 I'm not sure if it matters, but you could shorten these two lines to
 just one:

 return virHashCodeGen(name, sizeof(unsigned long), seed);
 
 I actually preferred the slightly more verbose style, to make
 it clearer that we're actually passing in the PID as an int,
 cast to a pointer, instead of an int pointer.

And given the recent thread about mingw32, this is wrong anyways.
There, 'unsigned long' is 32 bits, but pid_t is 64 bits, so we'd be
losing information in our hash.  We probably need to fix this code to be
passing (void *)pid_t and dereferencing the pointer to get to a full
pid, rather than cheating with (void*)(unsigned long)pid_t and possibly
losing bits (although since this is in a context of hashing, lost bits
only means more chance for collision, and not a fatal algorithmic flaw).


 where existing calls to virRandom(256) will now be virRandomBits(8),
 virRandom(1024*1024*1024) will now be virRandomBits(30), and your code
 would use uint32_t seed = virRandom(32).

Of course, here I meant virRandomBits(32),

 
 Yep, I've done that.

but I'm sure you figured that out.

 +
 +return le32toh(r);

 endian.h, and thus le32toh(), is not yet standardized (although POSIX
 will be adding it in the future), nor is it currently provided by
 gnulib.  We'd have to get that fixed first.
 
 The le32toh call was only here because the code I copied wanted to be
 endian neutral. I don't think libvirt really cares if its hash codes
 are endian neutral, so I trivially just removed the le32toh call and
 avoid the problem of endian.h

Agreed - we aren't sharing hash values over the wire, so all hash values
within a particular libvirtd process will be the same endianness,
without having to waste time on swapping bytes around.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 2/6] conf: put hostdev subsys data into a struct for easier re-use

2012-01-25 Thread Laine Stump
When interface is expanded to allow passthrough with extra
dev-dependent network interface info, the host-side address of the
device will need to be added to virDomainNetDef. It will be much
simpler if this is done with a common typedefed struct rather than
inlining the same struct stuff.
---
 src/conf/domain_conf.h |   34 +-
 1 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3b522a9..dd2abf8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -92,6 +92,26 @@ struct _virDomainDevicePCIAddress {
 int  multi;  /* enum virDomainDeviceAddressPciMulti */
 };
 
+typedef struct _virDomainDeviceSubsysUSBAddress 
virDomainDeviceSubsysUSBAddress;
+typedef virDomainDeviceSubsysUSBAddress *virDomainDeviceSubsysUSBAddressPtr;
+struct _virDomainDeviceSubsysUSBAddress {
+unsigned bus;
+unsigned device;
+
+unsigned vendor;
+unsigned product;
+};
+
+typedef struct _virDomainDeviceSubsysAddress virDomainDeviceSubsysAddress;
+typedef virDomainDeviceSubsysAddress *virDomainDeviceSubsysAddressPtr;
+struct _virDomainDeviceSubsysAddress {
+int type; /* enum virDomainHostdevBusType */
+union {
+virDomainDeviceSubsysUSBAddress usb;
+virDomainDevicePCIAddress pci; /* host address */
+} u;
+};
+
 typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress;
 typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr;
 struct _virDomainDeviceDriveAddress {
@@ -1088,19 +1108,7 @@ struct _virDomainHostdevDef {
 int mode; /* enum virDomainHostdevMode */
 unsigned int managed : 1;
 union {
-struct {
-int type; /* enum virDomainHostdevBusType */
-union {
-struct {
-unsigned bus;
-unsigned device;
-
-unsigned vendor;
-unsigned product;
-} usb;
-virDomainDevicePCIAddress pci; /* host address */
-} u;
-} subsys;
+virDomainDeviceSubsysAddress subsys; /* USB or PCI */
 struct {
 /* TBD: struct capabilities see:
  * 
https://www.redhat.com/archives/libvir-list/2008-July/msg00429.html
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 5/6] qemu: (and conf) support rombar for network devices

2012-01-25 Thread Laine Stump
When support for the rombar option was added, it was only added for
PCI passthrough devices, configured with hostdev. The same option is
available for any network device that is attached to the guest's PCI
bus. This patch allows setting rombar for any PCI network device type.

After adding cases to test this to qemuxml2argv-hostdev-pci-rombar.*,
I decided to rename those files (to qemuxml2argv-pci-rom.*) to more
accurately reflect the additional tests, and also noticed that up to
now we've only been performing a domainschematest for that case, so I
added the pci-rom test to both qemuxml2argv and qemuxml2xml (and in
the process found some bugs whose fixes I squashed into previous
commits of this series).
---
 docs/formatdomain.html.in  |   27 ++
 docs/schemas/domaincommon.rng  |   25 ++---
 src/conf/domain_conf.c |6 ++-
 src/qemu/qemu_command.c|   54 +---
 .../qemuxml2argv-hostdev-pci-rombar.args   |5 --
 tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args   |   12 
 ...dev-pci-rombar.xml = qemuxml2argv-pci-rom.xml} |   18 +++
 tests/qemuxml2argvtest.c   |3 +
 tests/qemuxml2xmltest.c|1 +
 9 files changed, 116 insertions(+), 35 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args
 rename tests/qemuxml2argvdata/{qemuxml2argv-hostdev-pci-rombar.xml = 
qemuxml2argv-pci-rom.xml} (57%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dfb010d..85cdbea 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1951,6 +1951,7 @@
   lt;mac address='00:16:3e:5d:c7:9e'/gt;
   lt;script path='vif-bridge'/gt;
   lt;boot order='1'/gt;
+  lt;rom bar='off'/gt;
 lt;/interfacegt;
   lt;/devicesgt;
   .../pre
@@ -2479,6 +2480,32 @@ qemu-kvm -net nic,model=? /dev/null
   span class=sinceSince 0.8.8/span
 /p
 
+h5a name=elementsNICSROMInterface ROM BIOS configuration/a/h5
+
+pre
+  ...
+  lt;devicesgt;
+lt;interface type='network'gt;
+  lt;source network='default'/gt;
+  lt;target dev='vnet1'/gt;
+  blt;rom bar='off'/gt;/b
+lt;/interfacegt;
+  lt;/devicesgt;
+  .../pre
+
+p
+  For hypervisors which support this, you can change how a PCI Network
+  device's ROM is presented to the guest. The codebar/code
+  attribute can be set to on or off, and determines whether
+  or not the device's ROM will be visible in the guest's memory
+  map. (In PCI documentation, the rombar setting controls the
+  presence of the Base Address Register for the ROM). If no rom
+  bar is specified, the qemu default will be used (older
+  versions of qemu used a default of off, while newer qemus
+  have a default of on). span class=sinceSince
+  0.9.10 (QEMU and KVM only)/span
+/p
+
 h5a name=elementQoSQuality of service/a/h5
 
 pre
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4fa968d..6bef645 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1490,6 +1490,9 @@
 ref name=deviceBoot/
   /optional
   optional
+ref name=rom/
+  /optional
+  optional
 ref name=bandwidth/
   /optional
 /interleave
@@ -2347,15 +2350,7 @@
 ref name=address/
   /optional
   optional
-element name=rom
-  attribute name=bar
-choice
-  valueon/value
-  valueoff/value
-/choice
-  /attribute
-  empty/
-/element
+ref name=rom/
   /optional
 /element
   /define
@@ -2815,6 +2810,18 @@
 /element
   /define
 
+  define name=rom
+element name=rom
+  attribute name=bar
+choice
+  valueon/value
+  valueoff/value
+/choice
+  /attribute
+  empty/
+/element
+  /define
+
   define name=usbmaster
 element name=master
   attribute name=startport
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9723d95..0799c5e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3875,7 +3875,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
 def-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
 } else {
 if (virDomainDeviceInfoParseXML(node, bootMap, def-info,
-flags | 
VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT)  0)
+flags | 
VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT
+| VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)  
0)
 goto error;
 }
 
@@ -10505,7 +10506,8 @@ virDomainNetDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(buf, -6);
 
 if (virDomainDeviceInfoFormat(buf, def-info,
- 

[libvirt] [PATCH 0/6] qemu: add suport for romfile, and associated cleanups

2012-01-25 Thread Laine Stump
This patchset started out as cleanup to make it easier to add in
support for doing PCI passthrough via the interface element (to be
used to attach a host network device to a guest after performing some
network-device-specific setup), but turned out to also make it much
easier to add support for QEMU's romfile option, as requested in 
https://bugzilla.redhat.com/show_bug.cgi?id=781562

Patch 1 is trivial, and probably qualifies for pushing under the
trivial rule, but it's just as easy to leave it in with the
rest. Patches 2-4 reorganize some data definitions, and the code
associated with them, with no functional change. Patch 5 expands
support for rombar to network devices (as long as they attach to the
guest PCI bus), and Patch 6 adds support for romfile to both generic
PCI devices and network devices.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/6] conf: remove duplicate call to VIR_FREE(info-alias)

2012-01-25 Thread Laine Stump
There is another identical call 4 lines up in the same function.
---
 src/conf/domain_conf.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e872396..471b0a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1807,7 +1807,6 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
 VIR_FREE(info-addr.usb.port);
 }
-VIR_FREE(info-alias);
 memset(info-addr, 0, sizeof(info-addr));
 info-type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
 }
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 6/6] qemu: add suport for romfile option to specify device boot ROM

2012-01-25 Thread Laine Stump
This patch addresses: https://bugzilla.redhat.com/show_bug.cgi?id=781562

Along with the rombar option that controls whether or not a boot rom
is made visible to the guest, qemu also has a romfile option that
allows specifying a binary file to present as the ROM BIOS of any
emulated or passthrough PCI device. This patch adds support for
specifying romfile to both passthrough PCI devices, and emulated
network devices that attach to the guest's PCI bus (just about
everything other than ne2k_isa).

One example of the usefulness of this option is described in the
bugzilla report: 82576 sriov network adapters don't provide a ROM BIOS
for the cards virtual functions (VF), but an image of such a ROM is
available, and with this ROM visible to the guest, it can PXE boot.

In libvirt's xml, the new option is configured like this:

   hostdev
 ...
 rom file='/etc/fake/boot.bin'/
 ...
   /hostdev

(similarly for interface).
---
 docs/formatdomain.html.in |   22 --
 docs/schemas/domaincommon.rng |   19 +--
 src/conf/domain_conf.c|   37 ++---
 src/conf/domain_conf.h|1 +
 src/qemu/qemu_command.c   |8 +---
 5 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 85cdbea..afd3d22 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1737,7 +1737,7 @@
 lt;address bus='0x06' slot='0x02' function='0x0'/gt;
   lt;/sourcegt;
   lt;boot order='1'/gt;
-  lt;rom bar='off'/gt;
+  lt;rom bar='on' file='/etc/fake/boot.bin'/gt;
 lt;/hostdevgt;
   lt;/devicesgt;
   .../pre
@@ -1779,7 +1779,7 @@
   span class=sinceSince 0.8.8/span/dd
   dtcoderom/code/dt
   ddThe coderom/code element is used to change how a PCI
-device's ROM is presented to the guest. The codebar/code
+device's ROM is presented to the guest. The optional codebar/code
 attribute can be set to on or off, and determines whether
 or not the device's ROM will be visible in the guest's memory
 map. (In PCI documentation, the rombar setting controls the
@@ -1787,7 +1787,13 @@
 bar is specified, the qemu default will be used (older
 versions of qemu used a default of off, while newer qemus
 have a default of on). span class=sinceSince
-0.9.7/span
+0.9.7 (QEMU and KVM only)/span. The optional
+codefile/code attribute is used to point to a binary file
+to be presented to the guest as the device's ROM BIOS. This
+can be useful, for example, to provide a PXE boot ROM for a
+virtual function of an sr-iov capable ethernet device (which
+has no boot ROMs for the VFs).
+span class=sinceSince 0.9.10 (QEMU and KVM only)/span.
   /dd
   dtcodeaddress/code/dt
   ddThe codeaddress/code element for USB devices has a
@@ -2488,7 +2494,7 @@ qemu-kvm -net nic,model=? /dev/null
 lt;interface type='network'gt;
   lt;source network='default'/gt;
   lt;target dev='vnet1'/gt;
-  blt;rom bar='off'/gt;/b
+  blt;rom bar='on' file='/etc/fake/boot.bin'/gt;/b
 lt;/interfacegt;
   lt;/devicesgt;
   .../pre
@@ -2502,8 +2508,12 @@ qemu-kvm -net nic,model=? /dev/null
   presence of the Base Address Register for the ROM). If no rom
   bar is specified, the qemu default will be used (older
   versions of qemu used a default of off, while newer qemus
-  have a default of on). span class=sinceSince
-  0.9.10 (QEMU and KVM only)/span
+  have a default of on).
+  The optional codefile/code attribute is used to point to a
+  binary file to be presented to the guest as the device's ROM
+  BIOS. This can be useful to provide an alternative boot ROM for a
+  network device.
+  span class=sinceSince 0.9.10 (QEMU and KVM only)/span.
 /p
 
 h5a name=elementQoSQuality of service/a/h5
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6bef645..5eea745 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2812,12 +2812,19 @@
 
   define name=rom
 element name=rom
-  attribute name=bar
-choice
-  valueon/value
-  valueoff/value
-/choice
-  /attribute
+  optional
+attribute name=bar
+  choice
+valueon/value
+valueoff/value
+  /choice
+/attribute
+  /optional
+  optional
+attribute name=file
+  ref name=absFilePath/
+/attribute
+  /optional
   empty/
 /element
   /define
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0799c5e..683417b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1811,6 +1811,7 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 }
 memset(info-addr, 0, sizeof(info-addr));
 info-type = 

[libvirt] [PATCH 4/6] conf: relocate rombar and boot order parse/format

2012-01-25 Thread Laine Stump
Since these two items are now in the virDomainDeviceInfo struct, it
makes sense to parse/format them in the functions written to
parse/format that structure. Not all types of devices allow them, so
two internal flags are added to indicate when it is appropriate to do
so.

I was lucky - only one test case needed to be re-ordered!
---
 src/conf/domain_conf.c |  224 +++-
 tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml |2 +-
 2 files changed, 123 insertions(+), 103 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a35e6e..9723d95 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -67,6 +67,8 @@ typedef enum {
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET = (117),
/* dump/parse original states of host PCI device */
VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (118),
+   VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (119),
+   VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (120),
 } virDomainXMLInternalFlags;
 
 VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
@@ -1908,6 +1910,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
   unsigned int flags)
 {
+if ((flags  VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT)  info-bootIndex)
+virBufferAsprintf(buf,   boot order='%d'/\n, info-bootIndex);
+
 if (info-alias 
 !(flags  VIR_DOMAIN_XML_INACTIVE)) {
 virBufferAsprintf(buf,   alias name='%s'/\n, info-alias);
@@ -1918,6 +1923,18 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   info-master.usb.startport);
 }
 
+if ((flags  VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)  info-rombar) {
+const char *rombar
+= virDomainPciRombarModeTypeToString(info-rombar);
+if (!rombar) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unexpected rom bar value %d),
+ info-rombar);
+return -1;
+}
+virBufferAsprintf(buf,   rom bar='%s'/\n, rombar);
+}
+
 if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
 return 0;
 
@@ -2266,11 +2283,56 @@ cleanup:
 return ret;
 }
 
+static int
+virDomainDeviceBootParseXML(xmlNodePtr node,
+int *bootIndex,
+virBitmapPtr bootMap)
+{
+char *order;
+int boot;
+int ret = -1;
+
+order = virXMLPropString(node, order);
+if (!order) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+%s, _(missing boot order attribute));
+goto cleanup;
+} else if (virStrToLong_i(order, NULL, 10, boot)  0 ||
+   boot = 0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+_(incorrect boot order '%s', expecting positive integer),
+order);
+goto cleanup;
+}
+
+if (bootMap) {
+bool set;
+if (virBitmapGetBit(bootMap, boot - 1, set)  0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(boot orders have to be contiguous and starting from 
1));
+goto cleanup;
+} else if (set) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+_(boot order %d used for more than one device), boot);
+goto cleanup;
+}
+ignore_value(virBitmapSetBit(bootMap, boot - 1));
+}
+
+*bootIndex = boot;
+ret = 0;
+
+cleanup:
+VIR_FREE(order);
+return ret;
+}
+
 /* Parse the XML definition for a device address
  * @param node XML nodeset to parse for device address definition
  */
 static int
 virDomainDeviceInfoParseXML(xmlNodePtr node,
+virBitmapPtr bootMap,
 virDomainDeviceInfoPtr info,
 unsigned int flags)
 {
@@ -2278,6 +2340,8 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
 xmlNodePtr address = NULL;
 xmlNodePtr master = NULL;
 xmlNodePtr alias = NULL;
+xmlNodePtr boot = NULL;
+xmlNodePtr rom = NULL;
 char *type = NULL;
 int ret = -1;
 
@@ -2296,6 +2360,14 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
 } else if (master == NULL 
xmlStrEqual(cur-name, BAD_CAST master)) {
 master = cur;
+} else if (boot == NULL  bootMap 
+   (flags  VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) 
+   xmlStrEqual(cur-name, BAD_CAST boot)) {
+boot = cur;
+} else if (rom == NULL 
+   (flags  VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) 
+   xmlStrEqual(cur-name, BAD_CAST rom)) {
+rom = cur;
 }
 }
 cur = cur-next;
@@ -2310,6 +2382,27 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
 goto cleanup;
 }
 
+if (boot) {
+if (virDomainDeviceBootParseXML(boot, 

Re: [libvirt] [PATCH] Replace hashing algorithm with murmurhash

2012-01-25 Thread Daniel P. Berrange
On Wed, Jan 25, 2012 at 09:55:25AM -0700, Eric Blake wrote:
 On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
  -static unsigned long virCgroupPidCode(const void *name)
  +static int32_t virCgroupPidCode(const void *name, int32_t seed)
   {
  -return (unsigned long)name;
  +unsigned long pid = (unsigned long)name;
  +return virHashCodeGen(pid, sizeof(pid), seed);
 
  I'm not sure if it matters, but you could shorten these two lines to
  just one:
 
  return virHashCodeGen(name, sizeof(unsigned long), seed);
  
  I actually preferred the slightly more verbose style, to make
  it clearer that we're actually passing in the PID as an int,
  cast to a pointer, instead of an int pointer.
 
 And given the recent thread about mingw32, this is wrong anyways.
 There, 'unsigned long' is 32 bits, but pid_t is 64 bits, so we'd be
 losing information in our hash.  We probably need to fix this code to be
 passing (void *)pid_t and dereferencing the pointer to get to a full
 pid, rather than cheating with (void*)(unsigned long)pid_t and possibly
 losing bits (although since this is in a context of hashing, lost bits
 only means more chance for collision, and not a fatal algorithmic flaw).

Actually given the context of this usage, we don't need to use pid_t
here. The values come from doing  scanf(%lu) on the cgroups task
file, and this will only ever run on a Linux host.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Replace hashing algorithm with murmurhash

2012-01-25 Thread Eric Blake
On 01/25/2012 09:55 AM, Eric Blake wrote:
 +
 +return le32toh(r);

 endian.h, and thus le32toh(), is not yet standardized (although POSIX
 will be adding it in the future), nor is it currently provided by
 gnulib.  We'd have to get that fixed first.

 The le32toh call was only here because the code I copied wanted to be
 endian neutral. I don't think libvirt really cares if its hash codes
 are endian neutral, so I trivially just removed the le32toh call and
 avoid the problem of endian.h
 
 Agreed - we aren't sharing hash values over the wire, so all hash values
 within a particular libvirtd process will be the same endianness,
 without having to waste time on swapping bytes around.

Actually, the more I think about this, the more I have to wonder:  Does
the incoming alignment affect the output hash?  That is, if I do

int i;
char array[12];
for (i = 0; i  4; i++) {
strcpy(array + i, 12345678);
printf(%x\n, (int) virHashStrCode(array + i, 0));
}

do I get the same values for all four iterations, on both little- and
big-endian architectures?  If not, then the byte-rearranging really is
important to the algorithm (that is, the algorithm is operating on
4-byte quantities, but must build up those quantities from 1-byte
quantities regardless of starting alignment, so endianness looks like it
plays a role in doing the conversion correctly).

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] storage: Fix any VolLookupByPath if we have an empty logical pool

2012-01-25 Thread Cole Robinson
On F16 at least, empty volume groups don't have a directory under /dev.
The directory only appears once a logical volume is created.

This tickles some behavior in BackendStablePath which ends with
libvirt sleeping for 5 seconds while waiting for the directory to appear.
This causes all sorts of problems for the virStorageVolLookupByPath API
which virtinst uses, even if trying to resolve a path that is independent
of the logical pool.

In reality we don't even need to do that checking since logical pools
always have a stable target path. Short circuit the polling in that
case.

Fixes bug 782261
---
 src/storage/storage_backend.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index d7394e0..306e487 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1347,6 +1347,10 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 if (!STRPREFIX(pool-def-target.path, /dev))
 goto ret_strdup;
 
+/* Logical pools are under /dev but already have stable paths */
+if (pool-def-type == VIR_STORAGE_POOL_LOGICAL)
+goto ret_strdup;
+
 /* We loop here because /dev/disk/by-{id,path} may not have existed
  * before we started this operation, so we have to give it some time to
  * get created.
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/4] Introduce new API for generating random numbers

2012-01-25 Thread Eric Blake
On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The old virRandom() API was not generating good random numbers.
 Replace it with a new API virRandomBits which instead of being
 told the upper limit, gets told the number of bits of randomness
 required.
 
 * src/util/virrandom.c, src/util/virrandom.h: Add virRandomBits,
   and move virRandomInitialize
 * src/util/util.h, src/util/util.c: Delete virRandom and
   virRandomInitialize
 * src/libvirt.c, src/security/security_selinux.c,
   src/test/test_driver.c, src/util/iohelper.c: Update for
   changes from virRandom to virRandomBits
 * src/storage/storage_backend_iscsi.c: Remove bogus call
   to virRandomInitialize  convert to virRandomBits
 ---

 +++ b/src/storage/storage_backend_iscsi.c

 -
 -snprintf(temp_ifacename, sizeof(temp_ifacename), libvirt-iface-%08x,
 - virRandom(1024 * 1024 * 1024));
 +snprintf(temp_ifacename, sizeof(temp_ifacename), libvirt-iface-%08llx,
 + (unsigned long long)virRandomBits(30));

Hmm - the cast is indeed necessary if we don't want to rely on PRIx64,
thanks to the change in return type as we switch to the new function;
knowing we only have 30 bits, we could get away with a smaller cast, as in:
 %08x, (unsigned)virRandomBits(30)

but this is not enough of an issue to be worth changing.

 +
 +
 +uint64_t virRandomBits(int nbits)
 +{

You lost my documentation comments from my proposal:

/* Return an evenly distributed random number between [0,2^nbits), where
nbits must be in the range (0,64]. */

Also, the tests failed to compile.  Squash this in, and then you have my
ACK:

diff --git i/tests/testutils.c w/tests/testutils.c
index acdfdc1..fccea17 100644
--- i/tests/testutils.c
+++ w/tests/testutils.c
@@ -1,7 +1,7 @@
 /*
  * testutils.c: basic test utils
  *
- * Copyright (C) 2005-2011 Red Hat, Inc.
+ * Copyright (C) 2005-2012 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -34,6 +34,7 @@
 #include buf.h
 #include logging.h
 #include command.h
+#include virrandom.h

 #if TEST_OOM_TRACE
 # include execinfo.h

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] storage: Fix any VolLookupByPath if we have an empty logical pool

2012-01-25 Thread Eric Blake
On 01/25/2012 11:15 AM, Cole Robinson wrote:
 On F16 at least, empty volume groups don't have a directory under /dev.
 The directory only appears once a logical volume is created.
 
 This tickles some behavior in BackendStablePath which ends with
 libvirt sleeping for 5 seconds while waiting for the directory to appear.
 This causes all sorts of problems for the virStorageVolLookupByPath API
 which virtinst uses, even if trying to resolve a path that is independent
 of the logical pool.
 
 In reality we don't even need to do that checking since logical pools
 always have a stable target path. Short circuit the polling in that
 case.
 
 Fixes bug 782261
 ---
  src/storage/storage_backend.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

ACK.  Isn't it funny how small a patch can be in proportion to the time
it took you to analyze why that one if statement is needed?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 2/4] Convert various virHash functions to use size_t / uint32

2012-01-25 Thread Eric Blake
On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 In preparation for conversion over to use the Murmurhash3
 algorithm, convert various virHash APIs to use size_t or
 uint32 for their return values/parameters, instead of the
 variable size 'unsigned long' or 'int' types
 ---
  src/util/cgroup.c |4 +-
  src/util/hash.c   |   56 +++-
  src/util/hash.h   |   14 ++--
  3 files changed, 38 insertions(+), 36 deletions(-)
 

 diff --git a/src/util/cgroup.c b/src/util/cgroup.c
 index 25f2691..a270965 100644
 --- a/src/util/cgroup.c
 +++ b/src/util/cgroup.c
 @@ -1636,9 +1636,9 @@ cleanup:
  }
  
  
 -static unsigned long virCgroupPidCode(const void *name)
 +static uint32_t virCgroupPidCode(const void *name)
  {
 -return (unsigned long)name;
 +return (uint32_t)(int64_t)name;

I know why you did the double cast - to shut up the compiler warnings
about converting a pointer to a different size int.  But you still get
that warning on 32-bit machines, unless you use:

s/int64_t/intptr_t/

Meanwhile, your argument about this hash code only being used on Linux
with cgroups, and where pid is not a pid_t but a parsed long int from
procfs, convinced me - the naming is a bit unfortunate, but there is no
bug that will impact mingw64 pid_t cleanups.

Again, you forgot 'make check'.  ACK if you fix the above cast, and
squash this in:

diff --git i/tests/hashtest.c w/tests/hashtest.c
index 441672c..e4b2bc1 100644
--- i/tests/hashtest.c
+++ w/tests/hashtest.c
@@ -40,7 +40,7 @@ testHashInit(int size)
 }

 if (virHashTableSize(hash) != oldsize  virTestGetDebug()) {
-fprintf(stderr, \nhash grown from %d to %d,
+fprintf(stderr, \nhash grown from %d to %zd,
 oldsize, virHashTableSize(hash));
 }
 }
@@ -75,7 +75,7 @@ testHashCheckCount(virHashTablePtr hash, int count)
 int iter_count = 0;

 if (virHashSize(hash) != count) {
-testError(\nhash contains %d instead of %d elements\n,
+testError(\nhash contains %zd instead of %d elements\n,
   virHashSize(hash), count);
 return -1;
 }

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 3/4] Rename hash.h and hash.c to virhash.h and virhash.c

2012-01-25 Thread Eric Blake
On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 In preparation for the patch to include Murmurhash3, which
 introduces a virhashcode.h and virhashcode.c files, rename
 the existing hash.h and hash.c to virhash.h and virhash.c
 respectively.
 ---
  po/POTFILES.in  |2 +-
  src/Makefile.am |2 +-
  src/conf/domain_conf.h  |2 +-
  src/conf/nwfilter_conf.h|2 +-
  src/conf/nwfilter_params.h  |2 +-
  src/cpu/cpu_generic.c   |2 +-
  src/qemu/qemu_monitor.h |2 +-
  src/qemu/qemu_monitor_text.h|1 -
  src/uml/uml_conf.h  |2 +-
  src/util/cgroup.c   |2 +-
  src/util/{hash.c = virhash.c}  |2 +-
  src/util/{hash.h = virhash.h}  |0
  src/xen/xen_driver.h|2 +-
  src/xen/xm_internal.c   |2 +-
  tests/Makefile.am   |   10 +-
  tests/{hashdata.h = virhashdata.h} |0
  tests/{hashtest.c = virhashtest.c} |4 ++--

Gotta love git diffs of a file rename :)

 +++ b/src/qemu/qemu_monitor_text.h
 @@ -28,7 +28,6 @@
  # include internal.h
  
  # include qemu_monitor.h
 -# include hash.h
  

Huh - wonder why we had a stale header.

Recently, there was talk on the gnulib list about generalizing the
useless-include syntax checker to be extensible to arbitrary headers,
although there is nothing yet concrete from that discussion.  If it ever
materializes, we could use it for our own headers to catch dead includes
like this one.

 diff --git a/src/util/hash.c b/src/util/virhash.c
 similarity index 99%
 rename from src/util/hash.c
 rename to src/util/virhash.c

Missing a change in the comment header.

ACK - here's my trivial contribution:

diff --git i/src/util/virhash.c w/src/util/virhash.c
index 7294c7e..8eff50b 100644
--- i/src/util/virhash.c
+++ w/src/util/virhash.c
@@ -1,9 +1,9 @@
 /*
- * hash.c: chained hash tables for domain and domain/connection
deallocations
+ * virhash.c: chained hash tables for domain and domain/connection
deallocations
  *
  * Reference: Your favorite introductory book on algorithms
  *
- * Copyright (C) 2011 Red Hat, Inc.
+ * Copyright (C) 2011-2012 Red Hat, Inc.
  * Copyright (C) 2000 Bjorn Reese and Daniel Veillard.
  *
  * Permission to use, copy, modify, and distribute this software for any


-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 3/4] Rename hash.h and hash.c to virhash.h and virhash.c

2012-01-25 Thread Eric Blake
On 01/25/2012 11:44 AM, Eric Blake wrote:
 On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 In preparation for the patch to include Murmurhash3, which
 introduces a virhashcode.h and virhashcode.c files, rename
 the existing hash.h and hash.c to virhash.h and virhash.c
 respectively.

 ACK - here's my trivial contribution:

Also, we probably ought to continue to ignore the old (stale) binary, as
well as the new binary name:

diff --git i/.gitignore w/.gitignore
index 61a9a38..2533fce 100644
--- i/.gitignore
+++ w/.gitignore
@@ -74,14 +74,15 @@
 /tests/hashtest
 /tests/jsontest
 /tests/networkxml2argvtest
 /tests/nwfilterxml2xmltest
 /tests/openvzutilstest
 /tests/qemuxmlnstest
 /tests/shunloadtest
+/tests/virhashtest
 /update.log
 Makefile
 Makefile.in
 TAGS
 coverage
 cscope.files
 cscope.out

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] storage: Fix any VolLookupByPath if we have an empty logical pool

2012-01-25 Thread Cole Robinson
On 01/25/2012 01:23 PM, Eric Blake wrote:
 On 01/25/2012 11:15 AM, Cole Robinson wrote:
 On F16 at least, empty volume groups don't have a directory under /dev.
 The directory only appears once a logical volume is created.

 This tickles some behavior in BackendStablePath which ends with
 libvirt sleeping for 5 seconds while waiting for the directory to appear.
 This causes all sorts of problems for the virStorageVolLookupByPath API
 which virtinst uses, even if trying to resolve a path that is independent
 of the logical pool.

 In reality we don't even need to do that checking since logical pools
 always have a stable target path. Short circuit the polling in that
 case.

 Fixes bug 782261
 ---
  src/storage/storage_backend.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 ACK.  Isn't it funny how small a patch can be in proportion to the time
 it took you to analyze why that one if statement is needed?
 

:)  Thanks Eric, pushed now.

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH libvirt] Add missing virGetGroupName()

2012-01-25 Thread Marc-André Lureau
Add missing function if !HAVE_GETPWUID_R.
---
 src/util/util.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index c00c2f9..33bcf29 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2471,6 +2471,15 @@ virSetUIDGID(uid_t uid ATTRIBUTE_UNUSED,
  %s, _(virSetUIDGID is not available));
 return -1;
 }
+
+char *
+virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
+{
+virUtilError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(virGetGroupName is not available));
+
+return NULL;
+}
 #endif /* HAVE_GETPWUID_R */
 
 
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 0/2] nwfilter: Compare filters for equality when updating

2012-01-25 Thread Stefan Berger
When a filter is updated, compare it and the original one for equality
so that unnecessary instantiations of rules can be avoided.

v3:
 - introducing a function to instantiate all running VMs' filters; call
   it upon nwfilter driver reload
 - reworked function comparing filters; determining whether two filters
   are equal now by comparing their XML representations

v2:
 - added a test case for testing virHashEqual

  Regards,
Stefan


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 2/2] nwfilter: Rebuild filters only if new filter is different than current

2012-01-25 Thread Stefan Berger
Compare two filters' XML for equality and only rebuild/instantiate the new
filter if the new and current filters are found to be different. This
improves performance during an update of a filter with no obvious change
or the reloading of filters during a 'kill -SIGHUP'

---
 src/conf/nwfilter_conf.c |   37 +
 1 file changed, 37 insertions(+)

Index: libvirt-iterator/src/conf/nwfilter_conf.c
===
--- libvirt-iterator.orig/src/conf/nwfilter_conf.c
+++ libvirt-iterator/src/conf/nwfilter_conf.c
@@ -2809,6 +2809,35 @@ virNWFilterTestUnassignDef(virConnectPtr
 return rc;
 }
 
+static bool
+virNWFilterDefEqual(const virNWFilterDefPtr def1, virNWFilterDefPtr def2,
+bool cmpUUIDs)
+{
+bool ret = false;
+unsigned char rem_uuid[VIR_UUID_BUFLEN];
+char *xml1, *xml2 = NULL;
+
+if (!cmpUUIDs) {
+/* make sure the UUIDs are equal */
+memcpy(rem_uuid, def2-uuid, sizeof(rem_uuid));
+memcpy(def2-uuid, def1-uuid, sizeof(def2-uuid));
+}
+
+if (!(xml1 = virNWFilterDefFormat(def1)) ||
+!(xml2 = virNWFilterDefFormat(def2)))
+goto cleanup;
+
+ret = STREQ(xml1, xml2);
+
+if (!cmpUUIDs)
+memcpy(def2-uuid, rem_uuid, sizeof(rem_uuid));
+
+cleanup:
+VIR_FREE(xml1);
+VIR_FREE(xml2);
+
+return ret;
+}
 
 virNWFilterObjPtr
 virNWFilterObjAssignDef(virConnectPtr conn,
@@ -2840,6 +2869,14 @@ virNWFilterObjAssignDef(virConnectPtr co
 virNWFilterLockFilterUpdates();
 
 if ((nwfilter = virNWFilterObjFindByName(nwfilters, def-name))) {
+
+if (virNWFilterDefEqual(def, nwfilter-def, false)) {
+virNWFilterDefFree(nwfilter-def);
+nwfilter-def = def;
+virNWFilterUnlockFilterUpdates();
+return nwfilter;
+}
+
 nwfilter-newDef = def;
 /* trigger the update on VMs referencing the filter */
 if (virNWFilterTriggerVMFilterRebuild(conn)) {

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v3 1/2] nwfilter: Force instantiation of filters upon driver reload

2012-01-25 Thread Stefan Berger
Introduce a function that rebuilds all running VMs' filters. Call
this function when reloading the nwfilter driver.

This addresses a problem introduced by the 2nd patch that typically
causes no filters to be reinstantiate anymore upon driver reload
since their XML has not changed. Yet the current behavior is that
upon a SIGHUP all filters get reinstantiated.

---
 src/conf/nwfilter_conf.c   |   23 +++
 src/conf/nwfilter_conf.h   |3 +++
 src/libvirt_private.syms   |1 +
 src/nwfilter/nwfilter_driver.c |2 ++
 src/nwfilter/nwfilter_gentech_driver.c |   12 +++-
 5 files changed, 40 insertions(+), 1 deletion(-)

Index: libvirt-iterator/src/conf/nwfilter_conf.c
===
--- libvirt-iterator.orig/src/conf/nwfilter_conf.c
+++ libvirt-iterator/src/conf/nwfilter_conf.c
@@ -2723,6 +2723,29 @@ virNWFilterCallbackDriversUnlock(void)
 
 static virHashIterator virNWFilterDomainFWUpdateCB;
 
+/**
+ * virNWFilterInstFiltersOnAllVMs:
+ * Apply all filters on all running VMs. Don't terminate in case of an
+ * error. This should be called upon reloading of the driver.
+ */
+int
+virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
+{
+int i;
+struct domUpdateCBStruct cb = {
+.conn = conn,
+.err = 0, /* ignored here */
+.step = STEP_APPLY_CURRENT,
+.skipInterfaces = NULL, /* not needed */
+};
+
+for (i = 0; i  nCallbackDriver; i++)
+callbackDrvArray[i]-vmFilterRebuild(conn,
+ virNWFilterDomainFWUpdateCB,
+ cb);
+
+return 0;
+}
 
 static int
 virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
Index: libvirt-iterator/src/conf/nwfilter_conf.h
===
--- libvirt-iterator.orig/src/conf/nwfilter_conf.h
+++ libvirt-iterator/src/conf/nwfilter_conf.h
@@ -577,6 +577,7 @@ enum UpdateStep {
 STEP_APPLY_NEW,
 STEP_TEAR_NEW,
 STEP_TEAR_OLD,
+STEP_APPLY_CURRENT,
 };
 
 struct domUpdateCBStruct {
@@ -722,6 +723,8 @@ void virNWFilterUnlockFilterUpdates(void
 int virNWFilterConfLayerInit(virHashIterator domUpdateCB);
 void virNWFilterConfLayerShutdown(void);
 
+int virNWFilterInstFiltersOnAllVMs(virConnectPtr conn);
+
 # define virNWFilterReportError(code, fmt...)  \
 virReportErrorHelper(VIR_FROM_NWFILTER, code, __FILE__,\
  __FUNCTION__, __LINE__, fmt)
Index: libvirt-iterator/src/nwfilter/nwfilter_driver.c
===
--- libvirt-iterator.orig/src/nwfilter/nwfilter_driver.c
+++ libvirt-iterator/src/nwfilter/nwfilter_driver.c
@@ -162,6 +162,8 @@ nwfilterDriverReload(void) {
 virNWFilterCallbackDriversUnlock();
 nwfilterDriverUnlock(driverState);
 
+virNWFilterInstFiltersOnAllVMs(conn);
+
 virConnectClose(conn);
 }
 
Index: libvirt-iterator/src/libvirt_private.syms
===
--- libvirt-iterator.orig/src/libvirt_private.syms
+++ libvirt-iterator/src/libvirt_private.syms
@@ -811,6 +811,7 @@ virNWFilterConfLayerShutdown;
 virNWFilterDefFormat;
 virNWFilterDefFree;
 virNWFilterDefParseString;
+virNWFilterInstFiltersOnAllVMs;
 virNWFilterJumpTargetTypeToString;
 virNWFilterLoadAllConfigs;
 virNWFilterLockFilterUpdates;
Index: libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c
===
--- libvirt-iterator.orig/src/nwfilter/nwfilter_gentech_driver.c
+++ libvirt-iterator/src/nwfilter/nwfilter_gentech_driver.c
@@ -1122,7 +1122,7 @@ virNWFilterDomainFWUpdateCB(void *payloa
 virDomainObjPtr obj = payload;
 virDomainDefPtr vm = obj-def;
 struct domUpdateCBStruct *cb = data;
-int i;
+int i, err;
 bool skipIface;
 
 virDomainObjLock(obj);
@@ -1156,6 +1156,16 @@ virNWFilterDomainFWUpdateCB(void *payloa
 cb-err = virNWFilterTearOldFilter(net);
 }
 break;
+
+case STEP_APPLY_CURRENT:
+err = virNWFilterInstantiateFilter(cb-conn,
+   vm-uuid,
+   net);
+if (err)
+virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+_(Failure while applying current filter on 
+VM %s), vm-name);
+break;
 }
 if (cb-err)
 break;

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] Replace hashing algorithm with murmurhash

2012-01-25 Thread Eric Blake
On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Recent discussions have illustrated the potential for DOS attacks
 with the hash table implementations used by most languages and
 libraries.
 
https://lwn.net/Articles/474912/
 
 libvirt has an internal hash table impl, and uses hash tables for
 a variety of purposes. The hash key generation code is pretty
 simple and thus not strongly collision resistant.
 
 This patch replaces the current libvirt hash key generator with
 the (public domain) Murmurhash3 code. In addition every hash
 table now gets a random seed value which is used to perturb the
 hashing code. This should make it impossible to mount any
 practical attack against libvirt hashing code.
 
 * bootstrap.conf: Import bitrotate module
 * src/Makefile.am: Add virhashcode.[ch]
 * src/util/util.c: Make virRandom() return a fixed 32 bit
   integer value.

This part of the commit message is stale.

 * src/util/hash.c, src/util/hash.h, src/util/cgroup.c: Replace
   hash code generation with a call to virHashCodeGen()
 * src/util/virhashcode.h, src/util/virhashcode.c: Add a new
   virHashCodeGen() API using the Murmurhash3 algorithm.
 ---
  bootstrap.conf |1 +
  src/Makefile.am|1 +
  src/util/cgroup.c  |6 ++-
  src/util/virhash.c |   21 +++--
  src/util/virhash.h |7 ++-
  src/util/virhashcode.c |  121 
 
  src/util/virhashcode.h |   35 ++
  7 files changed, 174 insertions(+), 18 deletions(-)
  create mode 100644 src/util/virhashcode.c
  create mode 100644 src/util/virhashcode.h
 

 @@ -1636,9 +1637,10 @@ cleanup:
  }
  
  
 -static uint32_t virCgroupPidCode(const void *name)
 +static uint32_t virCgroupPidCode(const void *name, uint32_t seed)
  {
 -return (uint32_t)(int64_t)name;
 +unsigned long pid = (unsigned long)name;
 +return virHashCodeGen(pid, sizeof(pid), seed);

Hmm - now we're losing the double-cast, so you might be reintroducing
compiler warnings about casting a pointer to an incorrectly-sized
integer.  I think you still need to use:

unsigned long pid = (unsigned long)(intptr_t)name;

 + *
 + * The hash code generation is based on the public domain MurmurHash3 from 
 Austin Appleby:
 + * http://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp
 + *
 + * We use only the 32 bit variant because the 2 produce different result 
 while

s/result/results/

 +/* slower than original but is endian neutral and handles platforms that
 + * do only aligned reads */

s/is endian neutral and/

Your version is endian-dependent, but that's okay (you convinced me that
what matters is that a single libvirtd instance will always generate the
same hash for the same string, regardless of alignment; and that we
aren't transferring the hash over the wire to any other libvirtd
instance running on a different-endian machine).

 +__attribute__((always_inline))
 +static uint32_t getblock(const uint8_t *p, int i)
 +{
 +uint32_t r;
 +size_t size = sizeof(uint32_t);

I would have used sizeof(r) here (it's always nicer to use sizeof(var)
rather than sizeof(type), when a var is present, in case var changes
type later in a later patch).

 +
 +memcpy(r, p[i * size], size);

Hopefully the compiler optimizes this decently.

 +
 +switch (len  3) {
 +case 3:
 +k1 ^= tail[2]  16;
 +case 2:
 +k1 ^= tail[1]  8;
 +case 1:

I'd add some /* fallthrough */ comments to shut up Coverity.

Beyond that, it looked like a faithful conversion of the upstream code.

ACK with nits addressed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt] Add missing virGetGroupName()

2012-01-25 Thread Eric Blake
On 01/25/2012 11:54 AM, Marc-André Lureau wrote:
 Add missing function if !HAVE_GETPWUID_R.
 ---
  src/util/util.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/src/util/util.c b/src/util/util.c
 index c00c2f9..33bcf29 100644
 --- a/src/util/util.c
 +++ b/src/util/util.c
 @@ -2471,6 +2471,15 @@ virSetUIDGID(uid_t uid ATTRIBUTE_UNUSED,
   %s, _(virSetUIDGID is not available));
  return -1;
  }
 +
 +char *
 +virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
 +{
 +virUtilError(VIR_ERR_INTERNAL_ERROR,
 + %s, _(virGetGroupName is not available));
 +
 +return NULL;
 +}
  #endif /* HAVE_GETPWUID_R */

ACK and pushed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Marc-André Lureau
windows.h is included by threads.h.
winsock2.h should be included before.

Avoid the following warning:

In file included from ../gnulib/lib/unistd.h:51:0,
 from ../src/util/util.h:30,
 from rpc/virkeepalive.c:29:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2: warning: 
#warning Please include winsock2.h before windows.h [-Wcpp]
---
 src/rpc/virkeepalive.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
index 06b8e63..f345b85 100644
--- a/src/rpc/virkeepalive.c
+++ b/src/rpc/virkeepalive.c
@@ -22,6 +22,10 @@
 
 #include config.h
 
+#ifdef HAVE_WINSOCK2_H
+# include winsock2.h
+#endif
+
 #include memory.h
 #include threads.h
 #include virfile.h
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64

2012-01-25 Thread Marc-André Lureau
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.

Unfortunately, gnu_printf attribute check expect %lld while normal
printf is PRId64. So one warning remains.
---
 src/rpc/virnetsocket.c |4 ++--
 src/util/command.c |   10 +-
 src/util/util.h|8 
 src/util/virpidfile.c  |6 +++---
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 67d33b7..20bee4b 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -114,7 +114,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr 
localAddr,
 virNetSocketPtr sock;
 int no_slow_start = 1;
 
-VIR_DEBUG(localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%d,
+VIR_DEBUG(localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=% PID_FORMAT,
   localAddr, remoteAddr,
   fd, errfd, pid);
 
@@ -174,7 +174,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr 
localAddr,
 sock-client = isClient;
 
 PROBE(RPC_SOCKET_NEW,
-  sock=%p refs=%d fd=%d errfd=%d pid=%d localAddr=%s, remoteAddr=%s,
+  sock=%p refs=%d fd=%d errfd=%d pid=% PID_FORMAT  localAddr=%s, 
remoteAddr=%s,
   sock, sock-refs, fd, errfd,
   pid, NULLSTR(sock-localAddrStr), NULLSTR(sock-remoteAddrStr));
 
diff --git a/src/util/command.c b/src/util/command.c
index dc3cfc5..1dc0090 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -2103,7 +2103,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
 
 if (cmd-pid != -1) {
 virCommandError(VIR_ERR_INTERNAL_ERROR,
-_(command is already running as pid %d),
+_(command is already running as pid % PID_FORMAT),
 cmd-pid);
 return -1;
 }
@@ -2178,7 +2178,7 @@ virPidWait(pid_t pid, int *exitstatus)
 int status;
 
 if (pid = 0) {
-virReportSystemError(EINVAL, _(unable to wait for process %d), pid);
+virReportSystemError(EINVAL, _(unable to wait for process % 
PID_FORMAT), pid);
 return -1;
 }
 
@@ -2187,7 +2187,7 @@ virPidWait(pid_t pid, int *exitstatus)
errno == EINTR);
 
 if (ret == -1) {
-virReportSystemError(errno, _(unable to wait for process %d), pid);
+virReportSystemError(errno, _(unable to wait for process % 
PID_FORMAT), pid);
 return -1;
 }
 
@@ -2195,7 +2195,7 @@ virPidWait(pid_t pid, int *exitstatus)
 if (status != 0) {
 char *st = virCommandTranslateStatus(status);
 virCommandError(VIR_ERR_INTERNAL_ERROR,
-_(Child process (%d) status unexpected: %s),
+_(Child process (% PID_FORMAT ) status 
unexpected: %s),
 pid, NULLSTR(st));
 VIR_FREE(st);
 return -1;
@@ -2351,7 +2351,7 @@ void
 virPidAbort(pid_t pid)
 {
 /* Not yet ported to mingw.  Any volunteers?  */
-VIR_DEBUG(failed to reap child %d, abandoning it, pid);
+VIR_DEBUG(failed to reap child % PID_FORMAT , abandoning it, pid);
 }
 
 void
diff --git a/src/util/util.h b/src/util/util.h
index 94d9282..01c29ea 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -31,6 +31,7 @@
 # include sys/select.h
 # include sys/types.h
 # include stdarg.h
+# include inttypes.h
 
 # ifndef MIN
 #  define MIN(a, b) ((a)  (b) ? (a) : (b))
@@ -39,6 +40,13 @@
 #  define MAX(a, b) ((a)  (b) ? (a) : (b))
 # endif
 
+#ifdef _WIN64
+/* XXX gnu_printf prefers lld while non-gnu printf expect PRId64... */
+# define PID_FORMAT lld
+#else
+# define PID_FORMAT d
+#endif
+
 ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
 ssize_t safewrite(int fd, const void *buf, size_t count)
 ATTRIBUTE_RETURN_CHECK;
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 1fd6318..f2532d4 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -69,7 +69,7 @@ int virPidFileWritePath(const char *pidfile,
 goto cleanup;
 }
 
-if (fprintf(file, %d, pid)  0) {
+if (fprintf(file, % PID_FORMAT, pid)  0) {
 rc = -errno;
 goto cleanup;
 }
@@ -127,7 +127,7 @@ int virPidFileReadPath(const char *path,
 goto cleanup;
 }
 
-if (fscanf(file, %d, pid) != 1) {
+if (fscanf(file, % PID_FORMAT, pid) != 1) {
 rc = -EINVAL;
 VIR_FORCE_FCLOSE(file);
 goto cleanup;
@@ -209,7 +209,7 @@ int virPidFileReadPathIfAlive(const char *path,
 }
 #endif
 
-if (virAsprintf(procpath, /proc/%d/exe, *pid)  0) {
+if (virAsprintf(procpath, /proc/% PID_FORMAT /exe, *pid)  0) {
 *pid = -1;
 return -1;
 }
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH libvirt 6/6] Cast timeval.tv_sec long to localtime expected type time_t

2012-01-25 Thread Marc-André Lureau
---
 tools/virsh.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 41c..246e638 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3703,7 +3703,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const 
char *mime)
 /* add mime type here */
 
 gettimeofday(cur_time, NULL);
-localtime_r(cur_time.tv_sec, time_info);
+localtime_r((time_t *)cur_time.tv_sec, time_info);
 strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info);
 
 if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom),
@@ -18101,7 +18101,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const 
char *msg_format,
  * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
 */
 gettimeofday(stTimeval, NULL);
-stTm = localtime(stTimeval.tv_sec);
+stTm = localtime((time_t *)stTimeval.tv_sec);
 virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] ,
   (1900 + stTm-tm_year),
   (1 + stTm-tm_mon),
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v3 1/2] nwfilter: Force instantiation of filters upon driver reload

2012-01-25 Thread Eric Blake
On 01/25/2012 11:58 AM, Stefan Berger wrote:
 Introduce a function that rebuilds all running VMs' filters. Call
 this function when reloading the nwfilter driver.
 
 This addresses a problem introduced by the 2nd patch that typically
 causes no filters to be reinstantiate anymore upon driver reload
 since their XML has not changed. Yet the current behavior is that
 upon a SIGHUP all filters get reinstantiated.
 
 ---
  src/conf/nwfilter_conf.c   |   23 +++
  src/conf/nwfilter_conf.h   |3 +++
  src/libvirt_private.syms   |1 +
  src/nwfilter/nwfilter_driver.c |2 ++
  src/nwfilter/nwfilter_gentech_driver.c |   12 +++-
  5 files changed, 40 insertions(+), 1 deletion(-)
 

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 1/4] add a qemu-specific event register API, to passthough the new events come from qemu

2012-01-25 Thread Eric Blake
On 12/16/2011 09:58 AM, shao...@linux.vnet.ibm.com wrote:
 From: ShaoHe Feng shao...@linux.vnet.ibm.com
 
 Basically, this feature can go along with qemu monitor passthrough.
 That way, if we use new commands in the monitor that generate new events, we 
 want some way to receive those new events too.
 
 Signed-off-by: ShaoHe Feng shao...@linux.vnet.ibm.com
 ---
  include/libvirt/libvirt-qemu.h |   27 
  include/libvirt/libvirt.h.in   |2 +-
  src/conf/domain_event.c|  293 
 ++--
  src/conf/domain_event.h|   50 ++-
  src/driver.h   |   14 ++
  src/libvirt-qemu.c |  189 ++
  src/libvirt_private.syms   |6 +
  src/libvirt_qemu.syms  |5 +
  8 files changed, 571 insertions(+), 15 deletions(-)

Where do we stand on this series?  It seems like something worth
including in 0.9.10; can we get it rebased to the latest libvirt so it
can get a good review?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 2/2] nwfilter: Rebuild filters only if new filter is different than current

2012-01-25 Thread Eric Blake
On 01/25/2012 11:58 AM, Stefan Berger wrote:
 Compare two filters' XML for equality and only rebuild/instantiate the new
 filter if the new and current filters are found to be different. This
 improves performance during an update of a filter with no obvious change
 or the reloading of filters during a 'kill -SIGHUP'
 
 ---
  src/conf/nwfilter_conf.c |   37 +
  1 file changed, 37 insertions(+)
 
 Index: libvirt-iterator/src/conf/nwfilter_conf.c
 ===
 --- libvirt-iterator.orig/src/conf/nwfilter_conf.c
 +++ libvirt-iterator/src/conf/nwfilter_conf.c
 @@ -2809,6 +2809,35 @@ virNWFilterTestUnassignDef(virConnectPtr
  return rc;
  }
  
 +static bool
 +virNWFilterDefEqual(const virNWFilterDefPtr def1, virNWFilterDefPtr def2,
 +bool cmpUUIDs)
 +{
 +bool ret = false;
 +unsigned char rem_uuid[VIR_UUID_BUFLEN];
 +char *xml1, *xml2 = NULL;
 +
 +if (!cmpUUIDs) {
 +/* make sure the UUIDs are equal */
 +memcpy(rem_uuid, def2-uuid, sizeof(rem_uuid));
 +memcpy(def2-uuid, def1-uuid, sizeof(def2-uuid));
 +}
 +
 +if (!(xml1 = virNWFilterDefFormat(def1)) ||
 +!(xml2 = virNWFilterDefFormat(def2)))
 +goto cleanup;
 +
 +ret = STREQ(xml1, xml2);
 +
 +if (!cmpUUIDs)
 +memcpy(def2-uuid, rem_uuid, sizeof(rem_uuid));
 +
 +cleanup:

Misplaced label.  You need to slide it up two lines, and unconditionally
call the memcpy() to undo things when !cmpUUIDs.

ACK with that fixed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 1/6] errcode is typedef by mingw, rename an argument name

2012-01-25 Thread Eric Blake
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
 Fixes the following warning:
 util/virterror.c:1242:31: warning: declaration of 'errcode' shadows a global 
 declaration [-Wshadow]

Which header is polluting the namespace with a non-standard name?

 ---
  src/util/virterror.c |8 
  1 files changed, 4 insertions(+), 4 deletions(-)

At any rate, ACK and pushed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Eric Blake
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
 windows.h is included by threads.h.
 winsock2.h should be included before.
 
 Avoid the following warning:
 
 In file included from ../gnulib/lib/unistd.h:51:0,
  from ../src/util/util.h:30,
  from rpc/virkeepalive.c:29:
 /usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2: warning: 
 #warning Please include winsock2.h before windows.h [-Wcpp]
 ---
  src/rpc/virkeepalive.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

NACK.  I agree that things should be fixed, but the fix should live in
the problematic threads.h inclusion pattern, not in the downstream
virkeepalive.c.

Does this alternate patch work for you?  (One of these days, I need to
install the mingw64 cross-compiler, so I can test it myself.)

diff --git i/src/util/threads-win32.h w/src/util/threads-win32.h
index 8109afd..1b23402 100644
--- i/src/util/threads-win32.h
+++ w/src/util/threads-win32.h
@@ -1,7 +1,7 @@
 /*
  * threads-win32.h basic thread synchronization primitives
  *
- * Copyright (C) 2009, 2011 Red Hat, Inc.
+ * Copyright (C) 2009, 2011-2012 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
@@ -21,6 +21,9 @@

 #include internal.h

+#ifdef HAVE_WINSOCK2_H
+# include winsock2.h
+#endif
 #include windows.h

 struct virMutex {

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Marc-André Lureau


- Mensaje original -
 On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
  windows.h is included by threads.h.
  winsock2.h should be included before.
  
  Avoid the following warning:
  
  In file included from ../gnulib/lib/unistd.h:51:0,
   from ../src/util/util.h:30,
   from rpc/virkeepalive.c:29:
  /usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2:
  warning: #warning Please include winsock2.h before windows.h
  [-Wcpp]
  ---
   src/rpc/virkeepalive.c |4 
   1 files changed, 4 insertions(+), 0 deletions(-)
 
 NACK.  I agree that things should be fixed, but the fix should live
 in
 the problematic threads.h inclusion pattern, not in the downstream
 virkeepalive.c.

I agree, but ff we follow that reasoning, windows.h should include winsock2.h, 
shouldn't it?

 Does this alternate patch work for you?  (One of these days, I need
 to
 install the mingw64 cross-compiler, so I can test it myself.)

Yes, that works too.

Btw, I don't mind testing patches, but using mingw64 is almost a 'yum install' 
away :) And you could help to fix the few remaining issues. The most worrying 
being the gnulib stat module, the rest should be in control.

 
 diff --git i/src/util/threads-win32.h w/src/util/threads-win32.h
 index 8109afd..1b23402 100644
 --- i/src/util/threads-win32.h
 +++ w/src/util/threads-win32.h
 @@ -1,7 +1,7 @@
  /*
   * threads-win32.h basic thread synchronization primitives
   *
 - * Copyright (C) 2009, 2011 Red Hat, Inc.
 + * Copyright (C) 2009, 2011-2012 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
 @@ -21,6 +21,9 @@
 
  #include internal.h
 
 +#ifdef HAVE_WINSOCK2_H
 +# include winsock2.h
 +#endif
  #include windows.h
 
  struct virMutex {
 
 --
 Eric Blake   ebl...@redhat.com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH libvirt 1/6] errcode is typedef by mingw, rename an argument name

2012-01-25 Thread Marc-André Lureau


- Mensaje original -
 On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
  Fixes the following warning:
  util/virterror.c:1242:31: warning: declaration of 'errcode' shadows
  a global declaration [-Wshadow]
 
 Which header is polluting the namespace with a non-standard name?

crtdefs.h
10:typedef int errcode;


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Eric Blake
On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
 NACK.  I agree that things should be fixed, but the fix should live
 in
 the problematic threads.h inclusion pattern, not in the downstream
 virkeepalive.c.
 
 I agree, but ff we follow that reasoning, windows.h should include 
 winsock2.h, shouldn't it?

If it were POSIX, the answer is yes - all POSIX headers are required to
be idempotent and self-contained, so any header that requires some other
header to be included first, instead of doing the prereq includes under
the hood itself, is broken.  And gnulib already does a great job of
fixing the (surprisingly large number of) systems that fail this POSIX
header litmus test.

But we're talking about Windows, so we have to follow Microsoft
conventions.  And if Microsoft requires winsock2.h to be included
before windows.h, then mingw64 is doing the right thing in mimicking
that convention; at which point, we can't insist on fixing
windows.h.  But we _can_ insist on fixing libvirt's use of Windows
headers so that _only_ the windows-specific files have to know about the
dependency ordering constraints, and all other libvirt files can use any
libvirt header without having to think about inherited dependency
constraints.  threads-win32.h is our windows-specific header that was
triggering the problem, so that's the point in the chain worth fixing,
so that normal files, like virkeepalive.c don't have to think about if
I include both threads.h and unistd.h, am I introducing an ordering
constraint on windows platforms?

 Does this alternate patch work for you?  (One of these days, I need
 to
 install the mingw64 cross-compiler, so I can test it myself.)
 
 Yes, that works too.

Glad to hear it; I've pushed the alternate patch.

 
 Btw, I don't mind testing patches, but using mingw64 is almost a 'yum 
 install' away :) 

What repo?  The mingw64 compiler isn't in Fedora 16 proper.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Marc-André Lureau
Hi

- Mensaje original -
 On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
  
  Btw, I don't mind testing patches, but using mingw64 is almost a
  'yum install' away :)
 
 What repo?  The mingw64 compiler isn't in Fedora 16 proper.
 

The Fedora mingw64 project repo: 
http://build1.openftd.org/fedora-cross/fedora-cross.repo

(I also build mine one for epel6 
http://repos.fedorapeople.org/repos/elmarco/mingw64/epel-mingw64.repo, although 
it is not maintained regularly)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] add a qemu-specific event register API, to passthough the new events come from qemu

2012-01-25 Thread Adam Litke
On Wed, Jan 25, 2012 at 02:16:14PM -0700, Eric Blake wrote:
 On 12/16/2011 09:58 AM, shao...@linux.vnet.ibm.com wrote:
  From: ShaoHe Feng shao...@linux.vnet.ibm.com
  
  Basically, this feature can go along with qemu monitor passthrough.
  That way, if we use new commands in the monitor that generate new events, 
  we want some way to receive those new events too.
  
  Signed-off-by: ShaoHe Feng shao...@linux.vnet.ibm.com
  ---
   include/libvirt/libvirt-qemu.h |   27 
   include/libvirt/libvirt.h.in   |2 +-
   src/conf/domain_event.c|  293 
  ++--
   src/conf/domain_event.h|   50 ++-
   src/driver.h   |   14 ++
   src/libvirt-qemu.c |  189 ++
   src/libvirt_private.syms   |6 +
   src/libvirt_qemu.syms  |5 +
   8 files changed, 571 insertions(+), 15 deletions(-)
 
 Where do we stand on this series?  It seems like something worth
 including in 0.9.10; can we get it rebased to the latest libvirt so it
 can get a good review?

Thanks for asking about this...

Shaohe is on vacation for two weeks so I will do my best to answer in his
absence.  I believe the process of moving the event code into libvirt_qemu.so
had increased code size, complexity, and duplication.  He is still looking at
improving the series and will be posting another RFC soon after he returns from
vacation.  Therefore, we'll have to punt this one to the next release.

-- 
Adam Litke a...@us.ibm.com
IBM Linux Technology Center

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h

2012-01-25 Thread Eric Blake
On 01/25/2012 03:42 PM, Marc-André Lureau wrote:
 Hi
 
 - Mensaje original -
 On 01/25/2012 03:16 PM, Marc-André Lureau wrote:

 Btw, I don't mind testing patches, but using mingw64 is almost a
 'yum install' away :)

 What repo?  The mingw64 compiler isn't in Fedora 16 proper.

 
 The Fedora mingw64 project repo: 
 http://build1.openftd.org/fedora-cross/fedora-cross.repo

Thanks; I've got the cross-compiler installed now, so I can at least
play with the gnulib fixes.  However, I'm running into:

$ sysroot=$HOME/builder/x86_64-w64-mingw32/sys-root
$ ../configure --build=x86_64-pc-linux --host=x86_64-w64-mingw32 \
 --prefix=$sysroot/mingw --enable-compile-warnings=error \
 --without-libvirtd --without-python \
 PKG_CONFIG_PATH=$sysroot/mingw/lib/pkgconfig
...
checking for xdrmem_create in -lportablexdr... no
checking for library containing xdrmem_create... no
configure: error: Cannot find a XDR library

that is, while there is mingw32-portablexdr in the fedora-cross repo,
there is no mingw64-portablexdr, which chokes up configure.  What
configure arguments are you using?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 3/6] Fix warnings about pid_t printf format on mingw64

2012-01-25 Thread Eric Blake
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
 Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
 
 Unfortunately, gnu_printf attribute check expect %lld while normal
 printf is PRId64. So one warning remains.
 ---
  src/rpc/virnetsocket.c |4 ++--
  src/util/command.c |   10 +-
  src/util/util.h|8 
  src/util/virpidfile.c  |6 +++---
  4 files changed, 18 insertions(+), 10 deletions(-)

This failed 'make syntax-check':

libvirt_unmarked_diagnostics
src/util/command.c-2198- PID_FORMAT )
status unexpected: %s),

which may be a flaw in cfg.mk rather than an actual bug, but still one
we should address.

Also, I'm not quite convinced on your approach.  While it's nice to hide
the type behind a macro:
  
 +#ifdef _WIN64
 +/* XXX gnu_printf prefers lld while non-gnu printf expect PRId64... */

Libvirt should not be using non-gnu printf.  What function call gave you
that warning, so that we can fix it to use gnu printf?

 +# define PID_FORMAT lld
 +#else
 +# define PID_FORMAT d
 +#endif

the decision should _not_ be based on _WIN64, but instead on a
configure-time test on the underlying type of pid_t.  And since _that_
gets difficult, I'd almost rather go with the simpler approach of:

% PRIdMAX, (intmax_t) pid

everywhere that we currently use

%d, pid

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 2/2] build: allow for 64-bit pid

2012-01-25 Thread Eric Blake
Convert daemon code to handle 64-bit pid_t (even though at the
moment, it is not compiled on mingw).

* daemon/remote.c (remoteDispatchAuthList)
(remoteDispatchAuthPolkit): Print pid_t via %lld.
---
 daemon/remote.c |   50 +-
 1 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 7f552a7..1ada146 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2039,20 +2039,22 @@ remoteDispatchAuthList(virNetServerPtr server 
ATTRIBUTE_UNUSED,
  * some piece of polkit isn't present/running
  */
 if (auth == VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
-if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid, 
callerPid)  0) {
+if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
+  callerPid)  0) {
 /* Don't do anything on error - it'll be validated at next
  * phase of auth anyway */
 virResetLastError();
 } else if (callerUid == 0) {
-char ident[100];
-rv = snprintf(ident, sizeof ident, pid:%d,uid:%d, callerPid, 
callerUid);
-if (rv  0 || rv  sizeof ident) {
+char *ident;
+if (virAsprintf(ident, pid:%lld,uid:%d,
+(long long) callerPid, callerUid) == 0) {
 VIR_INFO(Bypass polkit auth for privileged client %s,
  ident);
 if (virNetServerClientSetIdentity(client, ident)  0)
 virResetLastError();
 else
 auth = VIR_NET_SERVER_SERVICE_AUTH_NONE;
+VIR_FREE(ident);
 }
 rv = -1;
 }
@@ -2491,13 +2493,15 @@ remoteDispatchAuthPolkit(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 goto authfail;
 }

-VIR_INFO(Checking PID %d running as %d, callerPid, callerUid);
+VIR_INFO(Checking PID %lld running as %d,
+ (long long) callerPid, callerUid);

 virCommandAddArg(cmd, --process);
-virCommandAddArgFormat(cmd, %d, callerPid);
+virCommandAddArgFormat(cmd, %lld, (long long) callerPid);
 virCommandAddArg(cmd, --allow-user-interaction);

-if (virAsprintf(ident, pid:%d,uid:%d, callerPid, callerUid)  0) {
+if (virAsprintf(ident, pid:%lld,uid:%d,
+(long long) callerPid, callerUid)  0) {
 virReportOOMError();
 goto authfail;
 }
@@ -2507,16 +2511,16 @@ remoteDispatchAuthPolkit(virNetServerPtr server 
ATTRIBUTE_UNUSED,

 if (status != 0) {
 char *tmp = virCommandTranslateStatus(status);
-VIR_ERROR(_(Policy kit denied action %s from pid %d, uid %d: %s),
-  action, callerPid, callerUid, NULLSTR(tmp));
+VIR_ERROR(_(Policy kit denied action %s from pid %lld, uid %d: %s),
+  action, (long long) callerPid, callerUid, NULLSTR(tmp));
 VIR_FREE(tmp);
 goto authdeny;
 }
 PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
   client=%p auth=%d identity=%s,
   client, REMOTE_AUTH_POLKIT, ident);
-VIR_INFO(Policy allowed action %s from pid %d, uid %d,
- action, callerPid, callerUid);
+VIR_INFO(Policy allowed action %s from pid %lld, uid %d,
+ action, (long long) callerPid, callerUid);
 ret-complete = 1;

 virNetServerClientSetIdentity(client, ident);
@@ -2566,7 +2570,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
 PolKitResult pkresult;
 DBusError err;
 const char *action;
-char ident[100];
+char *ident = NULL;
 int rv = -1;
 struct daemonClientPrivate *priv =
 virNetServerClientGetPrivateData(client);
@@ -2585,18 +2589,20 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
 goto authfail;
 }

-if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid, 
callerPid)  0) {
+if (virNetServerClientGetUNIXIdentity(client, callerUid, callerGid,
+  callerPid)  0) {
 VIR_ERROR(_(cannot get peer socket identity));
 goto authfail;
 }

-rv = snprintf(ident, sizeof ident, pid:%d,uid:%d, callerPid, callerUid);
-if (rv  0 || rv = sizeof ident) {
-VIR_ERROR(_(Caller identity was too large %d:%d), callerPid, 
callerUid);
+if (virAsprintf(ident, pid:%lld,uid:%d,
+(long long) callerPid, callerUid)  0) {
+virReportOOMError();
 goto authfail;
 }

-VIR_INFO(Checking PID %d running as %d, callerPid, callerUid);
+VIR_INFO(Checking PID %lld running as %d,
+ (long long) callerPid, callerUid);
 dbus_error_init(err);
 if (!(pkcaller = 
polkit_caller_new_from_pid(virNetServerGetDBusConn(server),
 callerPid, err))) {
@@ -2649,24 +2655,26 @@ remoteDispatchAuthPolkit(virNetServerPtr server,
 polkit_caller_unref(pkcaller);
 

Re: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64

2012-01-25 Thread Eric Blake
On 01/25/2012 05:22 PM, Marc-André Lureau wrote:
 the decision should _not_ be based on _WIN64, but instead on a
 configure-time test on the underlying type of pid_t.  And since
 _that_
 gets difficult, I'd almost rather go with the simpler approach of:

 % PRIdMAX, (intmax_t) pid

 everywhere that we currently use

 %d, pid

 
 I thought about using that solution, but I prefer the format macro.

Yes, it would be nice if inttypes.h had a macro for id_t.  But even
then, pid_t, uid_t, and gid_t are not necessarily the same width as id_t
(id_t is necessarily as large as the other three, but they don't all
have to be the same size).  And it gets unwieldy fast if you start
introducing more macros for more types.

 Tbh, I wish some of these would be part of gnulib (perhaps some already are).

Sadly, no one has made a case for extended type macros in gnulib.  And
mingw points out the problem - we can't use lld nor PRIdMAX for pid_t,
since we don't know whether the code is targetting gnu printf, windows
printf, or a mix.  I really do think we're stuck with casting to (long
long) or (intmax_t) in this case :(

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 libvirt 5/6] Cast pointer to int using intptr_t

2012-01-25 Thread Eric Blake
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
 Fix a few warnings with mingw64 x86_64.
 ---
  src/util/logging.c   |8 
  src/util/threads-win32.c |2 +-
  2 files changed, 5 insertions(+), 5 deletions(-)

ACK and pushed.

What a shame that (intptr_t) is the only portable type for converting
between void* and integers; but it's the right thing to do.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] The quest for virStorageVolResize()

2012-01-25 Thread Zeeshan Ali (Khattak)
Hi everyone,
   In Boxes we'll need to change the size of the storage volumes (we
use qcow2 files) but turns out that there is no virStorageVolResize()
yet[1]. In my chat with Daniel on IRC, he mentioned that this would be
a trivial task so I thought I should try to do it myself. I've been
looking into this for several hours now and haven't gotten very far. I
guess Daniel overestimated my skills of deciphering complicated code.
:) Attached is my very much WIP patch that at least builds but doesn't
exactly work yet:

virsh # vol-resize 'Microsoft Windows XP.qcow2' '4G' gnome-boxes
error: Failed to change size of volume 'Microsoft Windows XP.qcow2' to 4G

error: this function is not supported by the connection driver:
virStorageVolResize
-

If anyone can have a look and tell me if I'm going anywhere towards
the right direction and what level of indirection I'm missing here,
that would be awesome!

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

[1] Yes, I know there is a virDomainBlockResize but thats limited to
running domains and I haven't yet figured if it can be nicely binded
in libvirt-glib. Probably we'll want one function in libvirt-glib that
will abstract both virDomainBlockResize() and virStorageVolResize().
diff --git a/src/driver.h b/src/driver.h
index 24636a4..4b0b3d9 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1252,6 +1252,10 @@ typedef int
unsigned long long offset,
unsigned long long length,
unsigned int flags);
+typedef int
+(*virDrvStorageVolResize) (virStorageVolPtr vol,
+   unsigned long long capacity,
+   unsigned int flags);
 
 typedef int
 (*virDrvStoragePoolIsActive)(virStoragePoolPtr pool);
@@ -1313,6 +1317,7 @@ struct _virStorageDriver {
 virDrvStorageVolGetInfo volGetInfo;
 virDrvStorageVolGetXMLDesc volGetXMLDesc;
 virDrvStorageVolGetPath volGetPath;
+virDrvStorageVolResize volResize;
 virDrvStoragePoolIsActive   poolIsActive;
 virDrvStoragePoolIsPersistent   poolIsPersistent;
 };
diff --git a/src/libvirt.c b/src/libvirt.c
index 7b8adf7..2901234 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -12791,6 +12791,7 @@ virStorageVolGetPath(virStorageVolPtr vol)
 return ret;
 }
 
+VIR_WARN(driver: %s,conn-storageDriver-name);
 virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
 
 error:
@@ -12798,6 +12799,51 @@ error:
 return NULL;
 }
 
+/**
+ * virStorageVolResize:
+ * @vol: pointer to storage volume
+ * @capacity: new capacity
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Changes the capacity of the storage volume @vol to @capacity. The new
+ * capacity must not exceed the sum of current capacity of the volume and
+ * remainining free space of its parent pool. Also the new capacity must
+ * be greater than or equal to current allocation of the volume.
+ *
+ * Returns 0 on success, or -1 on error.
+ */
+int
+virStorageVolResize(virStorageVolPtr vol,
+unsigned long long capacity,
+unsigned int flags)
+{
+virConnectPtr conn;
+VIR_DEBUG(vol=%p, vol);
+
+virResetLastError();
+
+if (!VIR_IS_STORAGE_VOL(vol)) {
+virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+
+conn = vol-conn;
+
+if (conn-storageDriver  conn-storageDriver-volResize) {
+int ret;
+ret = conn-storageDriver-volResize(vol, capacity, flags);
+if (!ret)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(vol-conn);
+return -1;
+}
 
 /**
  * virNodeNumOfDevices:
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 4ca7216..5155022 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -516,4 +516,9 @@ LIBVIRT_0.9.9 {
 virDomainSetNumaParameters;
 } LIBVIRT_0.9.8;
 
+LIBVIRT_0.9.10 {
+global:
+	virStorageVolResize;
+} LIBVIRT_0.9.9;
+
 #  define new API here using predicted next version number 
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 75ed676..a37bf7c 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -44,6 +44,11 @@ typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjP
 typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr pool,
  virStorageVolDefPtr origvol, virStorageVolDefPtr newvol,
  unsigned int flags);
+typedef int (*virStorageBackendVolumeResize)(virConnectPtr conn,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ 

Re: [libvirt] [PATCH 2/6] conf: put hostdev subsys data into a struct for easier re-use

2012-01-25 Thread Eric Blake
On 01/25/2012 09:58 AM, Laine Stump wrote:
 When interface is expanded to allow passthrough with extra
 dev-dependent network interface info, the host-side address of the
 device will need to be added to virDomainNetDef. It will be much
 simpler if this is done with a common typedefed struct rather than
 inlining the same struct stuff.
 ---
  src/conf/domain_conf.h |   34 +-
  1 files changed, 21 insertions(+), 13 deletions(-)

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 3/6] conf: move rombar and bootIndex into virDomainDeviceInfo

2012-01-25 Thread Eric Blake
On 01/25/2012 09:58 AM, Laine Stump wrote:
 To help consolidate the commonality between virDomainHostdevDef and
 virDomainInterface into as few members as possible (and because I
 think it makes sense), this patch moves the rombar and bootIndex
 members into the info member that is common to both (and to all the
 other structs that use them).
 
 It's a bit problematic that this gives rombar and bootIndex to many
 device types that don't use them, but this is already the case for the
 master and mastertype members of virDomainDeviceInfo, and is properly
 commented as such in the definition.
 
 Note that this opens the door to supporting rombar for other devices
 that are attached to the guest PCI bus - virtio-blk-pci,
 virtio-net-pci, various other network adapters - which which have that
 capability in qemu, but previously had no support in libvirt.

Looks like a reasonable move; the code itself is fine once we accept the
need for the refactoring.  ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 4/6] conf: relocate rombar and boot order parse/format

2012-01-25 Thread Eric Blake
On 01/25/2012 09:58 AM, Laine Stump wrote:
 Since these two items are now in the virDomainDeviceInfo struct, it
 makes sense to parse/format them in the functions written to
 parse/format that structure. Not all types of devices allow them, so
 two internal flags are added to indicate when it is appropriate to do
 so.
 
 I was lucky - only one test case needed to be re-ordered!
 ---
  src/conf/domain_conf.c |  224 
 +++-
  tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml |2 +-
  2 files changed, 123 insertions(+), 103 deletions(-)

ACK.  Nice use of internal flags to share logic while still keeping the
overall parsing and output the same.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 5/6] qemu: (and conf) support rombar for network devices

2012-01-25 Thread Eric Blake
On 01/25/2012 09:58 AM, Laine Stump wrote:
 When support for the rombar option was added, it was only added for
 PCI passthrough devices, configured with hostdev. The same option is
 available for any network device that is attached to the guest's PCI
 bus. This patch allows setting rombar for any PCI network device type.
 
 After adding cases to test this to qemuxml2argv-hostdev-pci-rombar.*,
 I decided to rename those files (to qemuxml2argv-pci-rom.*) to more
 accurately reflect the additional tests, and also noticed that up to
 now we've only been performing a domainschematest for that case, so I
 added the pci-rom test to both qemuxml2argv and qemuxml2xml (and in
 the process found some bugs whose fixes I squashed into previous
 commits of this series).
 ---
  docs/formatdomain.html.in  |   27 ++
  docs/schemas/domaincommon.rng  |   25 ++---
  src/conf/domain_conf.c |6 ++-
  src/qemu/qemu_command.c|   54 
 +---
  .../qemuxml2argv-hostdev-pci-rombar.args   |5 --
  tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args   |   12 
  ...dev-pci-rombar.xml = qemuxml2argv-pci-rom.xml} |   18 +++
  tests/qemuxml2argvtest.c   |3 +
  tests/qemuxml2xmltest.c|1 +

docs, rng, and tests all in one go!  I'm impressed :)

I didn't see anything fishy in here, so ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 5/6] qemu: (and conf) support rombar for network devices

2012-01-25 Thread Eric Blake
On 01/25/2012 08:08 PM, Eric Blake wrote:
 On 01/25/2012 09:58 AM, Laine Stump wrote:
 When support for the rombar option was added, it was only added for
 PCI passthrough devices, configured with hostdev. The same option is
 available for any network device that is attached to the guest's PCI
 bus. This patch allows setting rombar for any PCI network device type.

 After adding cases to test this to qemuxml2argv-hostdev-pci-rombar.*,
 I decided to rename those files (to qemuxml2argv-pci-rom.*) to more
 accurately reflect the additional tests, and also noticed that up to
 now we've only been performing a domainschematest for that case, so I
 added the pci-rom test to both qemuxml2argv and qemuxml2xml (and in
 the process found some bugs whose fixes I squashed into previous
 commits of this series).
 ---
  docs/formatdomain.html.in  |   27 ++
  docs/schemas/domaincommon.rng  |   25 ++---
  src/conf/domain_conf.c |6 ++-
  src/qemu/qemu_command.c|   54 
 +---
  .../qemuxml2argv-hostdev-pci-rombar.args   |5 --
  tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args   |   12 
  ...dev-pci-rombar.xml = qemuxml2argv-pci-rom.xml} |   18 +++
  tests/qemuxml2argvtest.c   |3 +
  tests/qemuxml2xmltest.c|1 +
 
 docs, rng, and tests all in one go!  I'm impressed :)
 
 I didn't see anything fishy in here, so ACK.

Actually,

 +  define name=rom
 +element name=rom
 +  attribute name=bar
 +choice
 +  valueon/value
 +  valueoff/value
 +/choice
 +  /attribute
 +  empty/
 +/element
 +  /define
 +

 +-device pci-assign,host=06:12.6,id=hostdev1,bus=pci.0,addr=0x6,rombar=1,\
 +romfile=/etc/fake/bootrom.bin \

 +interface type='user'
 +  mac address='52:54:00:24:a5:9e'/
 +  model type='virtio'/
 +  rom file='/etc/fake/bootrom.bin'/
 +/interface

You don't add rom file='' until patch 6/6, and the RNG schema doesn't
look like it supports file yet.

/me installs the series to see if this passes 'make check'...

153) qemuxml2argvdata/qemuxml2argv-pci-rom.xml... FAILED
xmllint --relaxng
/home/remote/eblake/libvirt/tests/../docs/schemas/domain.rng --noout
/home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml
Relax-NG validity error : Extra element devices in interleave
/home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml:15:
element devices: Relax-NG validity error : Element domain failed to
validate content
/home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml
fails to validate

It looks like you didn't quite rebase this for 'git bisect' purposes.
Would you mind rebasing one more time to move the rom file= stuff into
6/6 where the rng support was added?  Since it is just motion between
the two patches, and the end result is the same, no need to post a v2.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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 6/6] qemu: add suport for romfile option to specify device boot ROM

2012-01-25 Thread Eric Blake
s/suport/support in the subject

actually, that makes the subject a bit long; how about just:

s/suport for //

On 01/25/2012 09:58 AM, Laine Stump wrote:
 This patch addresses: https://bugzilla.redhat.com/show_bug.cgi?id=781562

Always fun to finally get to the real driver of the series.

 
 Along with the rombar option that controls whether or not a boot rom
 is made visible to the guest, qemu also has a romfile option that
 allows specifying a binary file to present as the ROM BIOS of any
 emulated or passthrough PCI device. This patch adds support for
 specifying romfile to both passthrough PCI devices, and emulated
 network devices that attach to the guest's PCI bus (just about
 everything other than ne2k_isa).
 
 One example of the usefulness of this option is described in the
 bugzilla report: 82576 sriov network adapters don't provide a ROM BIOS
 for the cards virtual functions (VF), but an image of such a ROM is
 available, and with this ROM visible to the guest, it can PXE boot.
 
 In libvirt's xml, the new option is configured like this:
 
hostdev
  ...
  rom file='/etc/fake/boot.bin'/
  ...
/hostdev
 
 (similarly for interface).
 ---
  docs/formatdomain.html.in |   22 --
  docs/schemas/domaincommon.rng |   19 +--
  src/conf/domain_conf.c|   37 ++---
  src/conf/domain_conf.h|1 +
  src/qemu/qemu_command.c   |8 +---
  5 files changed, 57 insertions(+), 30 deletions(-)

See my amended review of 5/6 about moving the specific tests for this
new XML into this patch.

Beyond those nits, ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] [Qemu-devel] libvirt doesn't work with qemu 1.0

2012-01-25 Thread Eric Blake
On 12/02/2011 01:32 PM, Anthony Liguori wrote:
 But we already have to call 'qemu -h' for other reasons; so we might as
 well be efficient and learn as much as possible from that result than by
 calling both 'qemu -h' and 'qemu -qmp ...', in order to probe what qemu
 supports.

 Also, 'qemu -qmp' doesn't work.  What's the proper syntax for invoking
 qemu in order to query QMP capabilities, but without also starting up a
 guest?
 
 
 anthony@titi:~/build/qemu$ (sleep 1; echo -e '{execute:
 qmp_capabilities}\n{execute: quit}') |
 x86_64-softmmu/qemu-system-x86_64 -qmp stdio -S -display none | head -1
 {QMP: {version: {qemu: {micro: 94, minor: 15, major: 0},
 package: }, capabilities: []}}

Doesn't work on 0.12 (the first version with qmp):

$ /usr/libexec/qemu-kvm -qmp stdio -S -display none
qemu-kvm: -display: invalid option
$ /usr/libexec/qemu-kvm -qmp stdio -S -nographic
chardev: opening backend stdio failed
qemu: could not open serial device 'stdio': Argument list too long

but if a temporary vnc port is okay, I was indeed able to get a list of
commands.

$ (sleep .1; printf
'{execute:qmp_capabilities}\n{execute:query-commands}\n{execute:quit}\n')
| /usr/libexec/qemu-kvm -qmp stdio -S
VNC server running on `127.0.0.1:5901'
{QMP: {version: {qemu: {micro: 1, minor: 12, major: 0},
package: (qemu-kvm-0.12.1.2)}, capabilities: []}}
{return: {}}
{return: [{name: block_stream}, {name: block_job_cancel}, ...

 
 The sleep 1 is due to a bug.  What I would suggest libvirt due is run
 this command but connect to the monitor properly, execute those two
 commands, and use the monitor greeting to figure out the version number.
 
 If you really want a command to invoke, we could also add a command line
 switch that took a QMP command or something like that.

Yes, that would be nice, especially since it would be the sort of patch
that would be worth backporting into distro qemu instances that insist
on staying at 0.12 (yes, I'm looking at RHEL).  That way, libvirt can
automatically decide if qmp is new enough [basically, if qemu is 0.15 or
newer, or if qemu is 0.12 through 0.14 _and_ has hmp passthrough
backported].  In other words, if 'qemu -h' says -qmp-command is
available, and I could run the above as:

qemu-kvm -qmp-command '{execute:query-commands}'

and have qemu automatically fill in the rest, it would be a bit easier
to use.

 Parsing help output is not the supported way of getting the version.  We
 happen to provide a nice, programmatic and stable interface for getting
 the version information.  Please use it.

I'm still stuck with the fact that with everything I tried, 0.12
temporarily opens a VNC port, and that there's a difference between 0.12
(-display none didn't exist) and newer versions; the best would be a
command sequence that works for all versions of qemu that have qmp, and
if it fails, then fall back to -h scraping.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
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] qemu: support qmp on RHEL/CentOS qemu

2012-01-25 Thread Eric Blake
I'm getting tired of remembering to backport RHEL-specific
patches when building upstream libvirt on RHEL 6.x or CentOS.
All the affected versions of RHEL qemu-kvm have backported
enough patches to a) make JSON useful, and b) modify the
-help text to mention libvirt as the preferred interface;
which means this string in the help output is a reliable
indicator that we can outsmart a strict version check,
even when upstream qemu 0.12 lacked the needed features.

* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags):
Recognize particular help string present when enough features were
backported to be worth using JSON.
* tests/qemuhelptest.c (mymain): Update tests accordingly.
---
 src/qemu/qemu_capabilities.c |   17 +
 tests/qemuhelptest.c |6 ++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 54757fc..c4bad6f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1104,7 +1104,8 @@ qemuCapsComputeCmdFlags(const char *help,
 if (strstr(help, -netdev)) {
 /* Disable -netdev on 0.12 since although it exists,
  * the corresponding netdev_add/remove monitor commands
- * do not, and we need them to be able todo hotplug */
+ * do not, and we need them to be able to do hotplug.
+ * But see below about RHEL build. */
 if (version = 13000)
 qemuCapsSet(flags, QEMU_CAPS_NETDEV);
 }
@@ -1169,12 +1170,20 @@ qemuCapsComputeCmdFlags(const char *help,
 /* While JSON mode was available in 0.12.0, it was too
  * incomplete to contemplate using. The 0.13.0 release
  * is good enough to use, even though it lacks one or
- * two features. The benefits of JSON mode now outweigh
- * the downside.
+ * two features. This is also true of versions of qemu
+ * built for RHEL, labeled 0.12.1, but with extra text
+ * in the help output that mentions that features were
+ * backported for libvirt. The benefits of JSON mode now
+ * outweigh the downside.
  */
 #if HAVE_YAJL
- if (version = 13000)
+if (version = 13000) {
 qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON);
+} else if (version = 12000 
+   strstr(help, libvirt)) {
+qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON);
+qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+}
 #endif

 if (version = 13000)
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 8802271..5ad55bc 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -354,9 +354,11 @@ mymain(void)
 QEMU_CAPS_MIGRATE_QEMU_UNIX,
 QEMU_CAPS_CHARDEV,
 QEMU_CAPS_ENABLE_KVM,
+QEMU_CAPS_MONITOR_JSON,
 QEMU_CAPS_BALLOON,
 QEMU_CAPS_DEVICE,
 QEMU_CAPS_SMP_TOPOLOGY,
+QEMU_CAPS_NETDEV,
 QEMU_CAPS_RTC,
 QEMU_CAPS_VHOST_NET,
 QEMU_CAPS_NO_KVM_PIT,
@@ -503,9 +505,11 @@ mymain(void)
 QEMU_CAPS_MIGRATE_QEMU_UNIX,
 QEMU_CAPS_CHARDEV,
 QEMU_CAPS_ENABLE_KVM,
+QEMU_CAPS_MONITOR_JSON,
 QEMU_CAPS_BALLOON,
 QEMU_CAPS_DEVICE,
 QEMU_CAPS_SMP_TOPOLOGY,
+QEMU_CAPS_NETDEV,
 QEMU_CAPS_RTC,
 QEMU_CAPS_VHOST_NET,
 QEMU_CAPS_NO_KVM_PIT,
@@ -559,8 +563,10 @@ mymain(void)
 QEMU_CAPS_CHARDEV,
 QEMU_CAPS_ENABLE_KVM,
 QEMU_CAPS_BALLOON,
+QEMU_CAPS_MONITOR_JSON,
 QEMU_CAPS_DEVICE,
 QEMU_CAPS_SMP_TOPOLOGY,
+QEMU_CAPS_NETDEV,
 QEMU_CAPS_RTC,
 QEMU_CAPS_VHOST_NET,
 QEMU_CAPS_NO_KVM_PIT,
-- 
1.7.7.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [libvirt-glib] Prefer 'for' over 'while'

2012-01-25 Thread Zeeshan Ali (Khattak)
From: Zeeshan Ali (Khattak) zeesha...@gnome.org

In this particular case 'for' seems like a more natural choice as then
we don't need to update the iterator (which we were forgetting to do and
causing a hang in Boxes).
---
 libvirt-gconfig/libvirt-gconfig-helpers.c |5 +
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c 
b/libvirt-gconfig/libvirt-gconfig-helpers.c
index c406a49..cc2e5cc 100644
--- a/libvirt-gconfig/libvirt-gconfig-helpers.c
+++ b/libvirt-gconfig/libvirt-gconfig-helpers.c
@@ -169,17 +169,14 @@ void gvir_config_xml_foreach_child(xmlNodePtr node,
 
 g_return_if_fail(iter_func != NULL);
 
-it = node-children;
-while (it != NULL) {
+for (it = node-children; it != NULL; it = it-next) {
 gboolean cont;
-xmlNodePtr next = it-next;
 
 if (xmlIsBlankNode(it))
 continue;
 cont = iter_func(it, opaque);
 if (!cont)
 break;
-it = next;
 }
 }
 
-- 
1.7.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] docs: fix virsh man page

2012-01-25 Thread Eric Blake
Typo introduced in commit 4e9953a.

* tools/virsh.pod (snapshot-create): Fix pod error.
---

Pushing under the trivial rule.
It bothers me that I have to use RHEL 5 to catch obvious errors,
and that newer perl is too lax to flag bugs like this.

 tools/virsh.pod |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index b6962cf..e1d8774 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2046,7 +2046,7 @@ used to represent properties of snapshots.
 =over 4

 =item Bsnapshot-create Idomain [Ixmlfile] {[I--redefine [I--current]]
-| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]
+| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]
 [I--quiesce]}

 Create a snapshot for domain Idomain with the properties specified in
-- 
1.7.7.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] qemu: require qmp on new enough qemu

2012-01-25 Thread Eric Blake
The qemu developers have made it clear that modern qemu will no
longer guarantee human monitor command stability; furthermore,
some features, such as async events, are only supported via qmp.
If we are compiled without support for handling JSON, we cannot
expect to sanely interact with modern qemu.

However, things must continue to build on RHEL 5, where qemu
is stuck at 0.10, and where yajl is not available.

Another benefit of this patch: future additions of new monitor
commands need only focus on qemu_monitor_json.c, instead of
also wasting time with qemu_monitor_text.c.

* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Report
error if yajl is missing but qemu requires qmp.
(qemuCapsParseHelpStr): Propagate error.
(qemuCapsExtractVersionInfo): Update caller.
* tests/qemuhelptest.c (testHelpStrParsing): Likewise.
---

Tested on Fedora 16, RHEL 6.2, and RHEL 5.8.

 src/qemu/qemu_capabilities.c |   33 -
 src/qemu/qemu_capabilities.h |3 ++-
 tests/qemuhelptest.c |2 +-
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c4bad6f..13a9cb0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -982,12 +982,13 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps)
 }


-static void
+static int
 qemuCapsComputeCmdFlags(const char *help,
 unsigned int version,
 unsigned int is_kvm,
 unsigned int kvm_version,
-virBitmapPtr flags)
+virBitmapPtr flags,
+bool check_yajl ATTRIBUTE_UNUSED)
 {
 const char *p;
 const char *fsdev;
@@ -1184,6 +1185,23 @@ qemuCapsComputeCmdFlags(const char *help,
 qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON);
 qemuCapsSet(flags, QEMU_CAPS_NETDEV);
 }
+#else
+/* Starting with qemu 0.15 and newer, upstream qemu no longer
+ * promised to keep the human interface stable, but request that
+ * we use QMP (the JSON interface) for everything.  If the user
+ * forgot to include YAJL libraries when building their own
+ * libvirt but is targetting a newer qemu, we are better off
+ * telling them to recompile (the spec file includes the
+ * dependency, so distros won't hit this).  */
+if (version = 15000 ||
+(version = 12000  strstr(help, libvirt))) {
+if (check_yajl) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(this qemu binary requires compilation with 
yajl));
+return -1;
+}
+qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+}
 #endif

 if (version = 13000)
@@ -1204,6 +1222,7 @@ qemuCapsComputeCmdFlags(const char *help,

 if (version = 11000)
 qemuCapsSet(flags, QEMU_CAPS_CPU_HOST);
+return 0;
 }

 /* We parse the output of 'qemu -help' to get the QEMU
@@ -1235,7 +1254,8 @@ int qemuCapsParseHelpStr(const char *qemu,
  virBitmapPtr flags,
  unsigned int *version,
  unsigned int *is_kvm,
- unsigned int *kvm_version)
+ unsigned int *kvm_version,
+ bool check_yajl)
 {
 unsigned major, minor, micro;
 const char *p = help;
@@ -1291,7 +1311,9 @@ int qemuCapsParseHelpStr(const char *qemu,

 *version = (major * 1000 * 1000) + (minor * 1000) + micro;

-qemuCapsComputeCmdFlags(help, *version, *is_kvm, *kvm_version, flags);
+if (qemuCapsComputeCmdFlags(help, *version, *is_kvm, *kvm_version,
+flags, check_yajl)  0)
+goto cleanup;

 strflags = virBitmapString(flags);
 VIR_DEBUG(Version %u.%u.%u, cooked version %u, flags %s,
@@ -1314,6 +1336,7 @@ fail:
 _(cannot parse %s version number in '%s'),
 qemu, p ? p : help);

+cleanup:
 VIR_FREE(p);

 return -1;
@@ -1455,7 +1478,7 @@ int qemuCapsExtractVersionInfo(const char *qemu, const 
char *arch,

 if (!(flags = qemuCapsNew()) ||
 qemuCapsParseHelpStr(qemu, help, flags,
- version, is_kvm, kvm_version) == -1)
+ version, is_kvm, kvm_version, true) == -1)
 goto cleanup;

 /* Currently only x86_64 and i686 support PCI-multibus. */
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8b009f9..18d6bc8 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -162,7 +162,8 @@ int qemuCapsParseHelpStr(const char *qemu,
  virBitmapPtr qemuCaps,
  unsigned int *version,
  unsigned int *is_kvm,
- unsigned int *kvm_version);
+ unsigned int *kvm_version,
+ bool check_yajl);
 int 

Re: [libvirt] [libvirt-glib] Prefer 'for' over 'while'

2012-01-25 Thread Philipp Hahn
Hello,

On Thursday 26 January 2012 06:10:28 Zeeshan Ali (Khattak) wrote:
 -it = node-children;
 -while (it != NULL) {
 +for (it = node-children; it != NULL; it = it-next) {
...
 -xmlNodePtr next = it-next;
...
  cont = iter_func(it, opaque);
...
 -it = next;
Your changed version only has the same behaviour, if the user-passed-in 
function iter_func() never changes it-next, which you can't guarentee here. 
You need to keep the next copy.

BYtE
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Using Libvirt to change the bridge a virtual network card of a running vm is connected to

2012-01-25 Thread Ralf Spenneberg
Hi,

I agree, that it would be nice, if I even could change the host network
backend, although we do not need this functionality. 
For our purpose we can go with the current host network backend and
connect that just to a different bridge. Essentially this is just a
brctl-invocation. Hendrik Schwartke, one of my co-workers, posted an
appropiate very low invasive patch yesterday on this list. 
http://www.mail-archive.com/libvir-list@redhat.com/msg50961.html

What do you think? Currently it does not work with virtual networks
created by libvirt but with external configured bridges. We are working
on that.

Kind regards,

Ralf

Am Freitag, den 20.01.2012, 17:56 + schrieb Daniel P. Berrange:
 On Fri, Jan 20, 2012 at 12:42:18PM -0500, Laine Stump wrote:
  On 01/20/2012 11:05 AM, Ralf Spenneberg wrote:
  Hi there,
  
  we are using libvirt to manage our VMs. We have several VMs connected to
  different bridges on the same physical KVM host. We need to change the
  bridge a virtual machine is connected to once in a while. Currently we
  are just using brctl to do that. Since this needs to be done over the
  network we would like to use the Libvirtd. Currently this does not seem
  possible. We have not found any interface or function within the libvirt
  library to implement this in a live VM. Only shutdown VMs may be changed
  via editing the XML representation.
  
  One way to do this would be to detach the network device, then
  reattach it with the configuration changed. Of course this would
  result in the guest seeing its network device disappear, then
  re-appear again, which is probably not what you want (although it's
  still a bit better than shutting down the entire guest).
 
 I think I've mentioned before that I'd like to see the
 
  virDomainUpdateDevice()
 
 API enhanced, so that you can change the network device backend on
 the fly without touching the guest device. It is probably already
 possible todo given current QEMU functionality for adding/removing
 host network backends - provided QEMU doesn't auto-kill the guest
 device when you remove the host device.
 
 
 Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list