Re: [libvirt] [PATCH] maint: simplify some syntax check exemptions

2014-07-23 Thread Martin Kletzander

On Fri, Jul 18, 2014 at 03:23:51PM -0600, Eric Blake wrote:

Commit 5028160 accidentally weakened the strtol prohibitions to
skip ALL files under src/util instead of the former situation of
just protecting util/virsexpr.c; even though NONE of the files
in that directory need any protection.

Shorten some long lines while at it.

* cfg.mk (exclude_file_name_regexp--sc_prohibit_strtol): No need
to exclude all of util.
(exclude_file_name_regexp--sc_prohibit_sprintf): Reduce long line.
(exclude_file_name_regexp--sc_prohibit_raw_allocation): Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---
cfg.mk | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 0559edd..10dd79b 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1065,7 +1065,7 @@ exclude_file_name_regexp--sc_prohibit_nonreentrant = \
  
^((po|tests)/|docs/.*(py|html\.in)|run.in$$|tools/wireshark/util/genxdrstub\.pl$$)

exclude_file_name_regexp--sc_prohibit_raw_allocation = \
-  
^(docs/hacking\.html\.in)|(src/util/viralloc\.[ch]|examples/.*|tests/securityselinuxhelper\.c|tests/vircgroupmock\.c|tools/wireshark/src/packet-libvirt.c)$$
+  
^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|vircgroupmock)\.c|tools/wireshark/src/packet-libvirt\.c)$$

exclude_file_name_regexp--sc_prohibit_readlink = \
  ^src/(util/virutil|lxc/lxc_container)\.c$$
@@ -1073,12 +1073,11 @@ exclude_file_name_regexp--sc_prohibit_readlink = \
exclude_file_name_regexp--sc_prohibit_setuid = ^src/util/virutil\.c$$

exclude_file_name_regexp--sc_prohibit_sprintf = \
-  
^(docs/hacking\.html\.in)|(examples/systemtap/.*stp)|(src/dtrace2systemtap\.pl)|(src/rpc/gensystemtap\.pl)|(tools/wireshark/util/genxdrstub\.pl)$$
+  ^(docs/hacking\.html\.in|.*stp|.pl)$$



This fails for me even though the regexp makes sense.  Either (a)
adding an asterisk before 'pl' (the same way as for
'.*stp') or (b) encapsulating the caret in the parentheses should
work, probably one of those you meant to do.

ACK with that changed,

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] spec: Enable sanlock on qemu_kvm_arches for RHEL

2014-07-23 Thread Jiri Denemark
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
 libvirt.spec.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 76e57aa..c035c68 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -260,12 +260,12 @@
 %endif
 
 # Enable sanlock library for lock management with QEMU
-# Sanlock is available only on x86_64 for RHEL
+# Sanlock is available only on arches where kvm is available for RHEL
 %if 0%{?fedora} = 16
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
-%if 0%{?rhel} = 6
-%ifarch x86_64
+%if 0%{?rhel}
+%ifarch %{qemu_kvm_arches}
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
 %endif
-- 
2.0.2

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


Re: [libvirt] [PATCHv4 0/2] lxc keep/drop capabilities

2014-07-23 Thread Gao feng
On 07/18/2014 04:02 PM, Cédric Bosdonnat wrote:
 Hi all,
 
 Even though the v3 has been ACKed (but not pushed), I rebased it on top of
 master before pushing it and did a few changes that are worth checking before.
 
  * Path 1 (feature) and 3 (doc) of the previous version merge merged together 
 as
suggested for another similar series.
 
  * virCgroupAllowDevice() has been changed to use negative major / minor 
 device
values to output '*'. I never saw any of them negative, but I don't have a
good knowledge of that.
 


ACK  Pushed, thanks!

 --
 Cedric
 
 
 Cédric Bosdonnat (2):
   lxc: allow to keep or drop capabilities
   lxc domain from xml: convert lxc.cap.drop
 
  docs/drvlxc.html.in|  47 
  docs/schemas/domaincommon.rng  | 207 ++
  src/conf/domain_conf.c | 126 ++-
  src/conf/domain_conf.h |  56 +
  src/libvirt_private.syms   |   3 +
  src/lxc/lxc_cgroup.c   |   8 +
  src/lxc/lxc_container.c| 241 
 +++--
  src/lxc/lxc_native.c   |  25 +++
  src/util/vircgroup.c   |  57 -
  src/util/vircgroup.h   |   2 +
  tests/domainschemadata/domain-caps-features.xml|  28 +++
  tests/lxcconf2xmldata/lxcconf2xml-blkiotune.xml|   2 +
  tests/lxcconf2xmldata/lxcconf2xml-cpusettune.xml   |   2 +
  tests/lxcconf2xmldata/lxcconf2xml-cputune.xml  |   2 +
  tests/lxcconf2xmldata/lxcconf2xml-idmap.xml|   2 +
  .../lxcconf2xmldata/lxcconf2xml-macvlannetwork.xml |   4 +
  tests/lxcconf2xmldata/lxcconf2xml-memtune.xml  |   2 +
  tests/lxcconf2xmldata/lxcconf2xml-nonenetwork.xml  |   4 +
  tests/lxcconf2xmldata/lxcconf2xml-nonetwork.xml|   2 +
  tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml  |   4 +
  tests/lxcconf2xmldata/lxcconf2xml-simple.xml   |   8 +
  tests/lxcconf2xmldata/lxcconf2xml-vlannetwork.xml  |   4 +
  22 files changed, 816 insertions(+), 20 deletions(-)
  create mode 100644 tests/domainschemadata/domain-caps-features.xml
 


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

Re: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns

2014-07-23 Thread Gao feng
On 07/14/2014 06:01 PM, Chen Hanxiao wrote:
 kernel commit 7dc5dbc879bd0779924b5132a48b731a0bc04a1e
 forbid us doing a fresh mount for sysfs
 when enable userns but disable netns.
 This patch will create a bind mount in this senario.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  src/lxc/lxc_container.c | 44 +---
  1 file changed, 33 insertions(+), 11 deletions(-)
 

Pushed, thanks!

 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index 4d89677..8a27215 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -815,10 +815,13 @@ static int lxcContainerSetReadOnly(void)
  }
  
  
 -static int lxcContainerMountBasicFS(bool userns_enabled)
 +static int lxcContainerMountBasicFS(bool userns_enabled,
 +bool netns_disabled)
  {
  size_t i;
  int rc = -1;
 +char* mnt_src = NULL;
 +int mnt_mflags;
  
  VIR_DEBUG(Mounting basic filesystems);
  
 @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
  bool bindOverReadonly;
  virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
  
 +/* When enable userns but disable netns, kernel will
 + * forbid us doing a new fresh mount for sysfs.
 + * So we had to do a bind mount for sysfs instead.
 + */
 +if (userns_enabled  netns_disabled 
 +STREQ(mnt-src, sysfs)) {
 +if (VIR_STRDUP(mnt_src, /sys)  0) {
 +goto cleanup;
 +}
 +mnt_mflags = MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY|MS_BIND;
 +} else {
 +if (VIR_STRDUP(mnt_src, mnt-src)  0) {
 +goto cleanup;
 +}
 +mnt_mflags = mnt-mflags;
 +}
 +
  VIR_DEBUG(Processing %s - %s,
 -  mnt-src, mnt-dst);
 +  mnt_src, mnt-dst);
  
  if (mnt-skipUnmounted) {
  char *hostdir;
 @@ -856,7 +876,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
  if (virFileMakePath(mnt-dst)  0) {
  virReportSystemError(errno,
   _(Failed to mkdir %s),
 - mnt-src);
 + mnt_src);
  goto cleanup;
  }
  
 @@ -867,24 +887,24 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
   * we mount the filesystem in read-write mode initially, and then do 
 a
   * separate read-only bind mount on top of that.
   */
 -bindOverReadonly = !!(mnt-mflags  MS_RDONLY);
 +bindOverReadonly = !!(mnt_mflags  MS_RDONLY);
  
  VIR_DEBUG(Mount %s on %s type=%s flags=%x,
 -  mnt-src, mnt-dst, mnt-type, mnt-mflags  ~MS_RDONLY);
 -if (mount(mnt-src, mnt-dst, mnt-type, mnt-mflags  ~MS_RDONLY, 
 NULL)  0) {
 +  mnt_src, mnt-dst, mnt-type, mnt_mflags  ~MS_RDONLY);
 +if (mount(mnt_src, mnt-dst, mnt-type, mnt_mflags  ~MS_RDONLY, 
 NULL)  0) {
  virReportSystemError(errno,
   _(Failed to mount %s on %s type %s 
 flags=%x),
 - mnt-src, mnt-dst, NULLSTR(mnt-type),
 - mnt-mflags  ~MS_RDONLY);
 + mnt_src, mnt-dst, NULLSTR(mnt-type),
 + mnt_mflags  ~MS_RDONLY);
  goto cleanup;
  }
  
  if (bindOverReadonly 
 -mount(mnt-src, mnt-dst, NULL,
 +mount(mnt_src, mnt-dst, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY, NULL)  0) {
  virReportSystemError(errno,
   _(Failed to re-mount %s on %s flags=%x),
 - mnt-src, mnt-dst,
 + mnt_src, mnt-dst,
   MS_BIND|MS_REMOUNT|MS_RDONLY);
  goto cleanup;
  }
 @@ -893,6 +913,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
  rc = 0;
  
   cleanup:
 +VIR_FREE(mnt_src);
  VIR_DEBUG(rc=%d, rc);
  return rc;
  }
 @@ -1643,7 +1664,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr 
 vmDef,
  goto cleanup;
  
  /* Mounts the core /proc, /sys, etc filesystems */
 -if (lxcContainerMountBasicFS(vmDef-idmap.nuidmap)  0)
 +if (lxcContainerMountBasicFS(vmDef-idmap.nuidmap,
 + !vmDef-nnets)  0)
  goto cleanup;
  
  /* Ensure entire root filesystem (except /.oldroot) is readonly */
 

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


Re: [libvirt] [PATCH] LXC: show used memory as 0 when domain is not active

2014-07-23 Thread Gao feng
On 07/17/2014 05:28 PM, Chen Hanxiao wrote:
 Before:
 virsh # dominfo chx3
 State:  shut off
 Max memory: 92160 KiB
 Used memory:92160 KiB
 
 After:
 virsh # dominfo container1
 State:  shut off
 Max memory: 92160 KiB
 Used memory:0 KiB
 
 Similar to qemu cases.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  src/lxc/lxc_driver.c   | 2 +-
  src/qemu/qemu_driver.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 


ACK  Pushed, thanks!

 diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
 index b7b4b02..f094f86 100644
 --- a/src/lxc/lxc_driver.c
 +++ b/src/lxc/lxc_driver.c
 @@ -584,7 +584,7 @@ static int lxcDomainGetInfo(virDomainPtr dom,
  
  if (!virDomainObjIsActive(vm)) {
  info-cpuTime = 0;
 -info-memory = vm-def-mem.cur_balloon;
 +info-memory = 0;
  } else {
  if (virCgroupGetCpuacctUsage(priv-cgroup, (info-cpuTime))  0) {
  virReportError(VIR_ERR_OPERATION_FAILED,
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 33541d3..01107cf 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2534,7 +2534,7 @@ static int qemuDomainGetInfo(virDomainPtr dom,
  info-memory = vm-def-mem.cur_balloon;
  }
  } else {
 -info-memory = vm-def-mem.cur_balloon;
 +info-memory = 0;
  }
  
  info-nrVirtCpu = vm-def-vcpus;
 

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


Re: [libvirt] [PATCHv4 0/2] lxc keep/drop capabilities

2014-07-23 Thread Peter Krempa
On 07/23/14 09:17, Gao feng wrote:
 On 07/18/2014 04:02 PM, Cédric Bosdonnat wrote:
 Hi all,

 Even though the v3 has been ACKed (but not pushed), I rebased it on top of
 master before pushing it and did a few changes that are worth checking 
 before.

  * Path 1 (feature) and 3 (doc) of the previous version merge merged 
 together as
suggested for another similar series.

  * virCgroupAllowDevice() has been changed to use negative major / minor 
 device
values to output '*'. I never saw any of them negative, but I don't have a
good knowledge of that.

 
 
 ACK  Pushed, thanks!

Please compile the code before pushing! This broke the build:


 CC   lxc/libvirt_driver_lxc_impl_la-lxc_cgroup.lo
In file included from /usr/include/cap-ng.h:27:0,
 from lxc/lxc_container.c:48:
lxc/lxc_container.c: In function 'lxcContainerDropCapabilities':
lxc/lxc_container.c:1951:35: error: comparison of unsigned expression = 0 is 
always true [-Werror=type-limits]
 if (!cap_valid(capsMapping[i]))
   ^
cc1: all warnings being treated as errors
make[3]: *** [lxc/libvirt_driver_lxc_impl_la-lxc_container.lo] Error 1
make[3]: *** Waiting for unfinished jobs
lxc/lxc_cgroup.c: In function 'virLXCCgroupSetupDeviceACL':
lxc/lxc_cgroup.c:354:9: error: jump skips variable initialization 
[-Werror=jump-misses-init]
 goto cleanup;
 ^
lxc/lxc_cgroup.c:456:2: note: label 'cleanup' defined here
  cleanup:
  ^
lxc/lxc_cgroup.c:357:9: note: 'capMknod' declared here
 int capMknod = def-caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD];
 ^
cc1: all warnings being treated as errors


Fixup patch comming soon.

Peter



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix build after 47e5b5ae3262f140955abd57bbb13337c65a3497

2014-07-23 Thread Peter Krempa
The patch described above introduced two problems caught by the compiler
and thus breaking the build.

One of the problems was comparison of unsigned with  0 and the second
one jumped a variable init.
---
 src/lxc/lxc_cgroup.c|  2 +-
 src/lxc/lxc_container.c | 74 -
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 572caca..1e96d72 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -337,6 +337,7 @@ virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev 
ATTRIBUTE_UNUSED,
 static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
   virCgroupPtr cgroup)
 {
+int capMknod = def-caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD];
 int ret = -1;
 size_t i;
 static virLXCCgroupDevicePolicy devices[] = {
@@ -354,7 +355,6 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
 goto cleanup;

 /* white list mknod if CAP_MKNOD has to be kept */
-int capMknod = def-caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD];
 if (capMknod == VIR_DOMAIN_FEATURE_STATE_ON) {
 if (virCgroupAllowAllDevices(cgroup,
 VIR_CGROUP_DEVICE_MKNOD)  0)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 49028be..81ef961 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1899,43 +1899,43 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def,
 int policy = def-features[VIR_DOMAIN_FEATURE_CAPABILITIES];

 /* Maps virDomainCapsFeature to CAPS_* */
-static unsigned int capsMapping[] = {CAP_AUDIT_CONTROL,
- CAP_AUDIT_WRITE,
- CAP_BLOCK_SUSPEND,
- CAP_CHOWN,
- CAP_DAC_OVERRIDE,
- CAP_DAC_READ_SEARCH,
- CAP_FOWNER,
- CAP_FSETID,
- CAP_IPC_LOCK,
- CAP_IPC_OWNER,
- CAP_KILL,
- CAP_LEASE,
- CAP_LINUX_IMMUTABLE,
- CAP_MAC_ADMIN,
- CAP_MAC_OVERRIDE,
- CAP_MKNOD,
- CAP_NET_ADMIN,
- CAP_NET_BIND_SERVICE,
- CAP_NET_BROADCAST,
- CAP_NET_RAW,
- CAP_SETGID,
- CAP_SETFCAP,
- CAP_SETPCAP,
- CAP_SETUID,
- CAP_SYS_ADMIN,
- CAP_SYS_BOOT,
- CAP_SYS_CHROOT,
- CAP_SYS_MODULE,
- CAP_SYS_NICE,
- CAP_SYS_PACCT,
- CAP_SYS_PTRACE,
- CAP_SYS_RAWIO,
- CAP_SYS_RESOURCE,
- CAP_SYS_TIME,
- CAP_SYS_TTY_CONFIG,
- CAP_SYSLOG,
- CAP_WAKE_ALARM};
+static int capsMapping[] = {CAP_AUDIT_CONTROL,
+CAP_AUDIT_WRITE,
+CAP_BLOCK_SUSPEND,
+CAP_CHOWN,
+CAP_DAC_OVERRIDE,
+CAP_DAC_READ_SEARCH,
+CAP_FOWNER,
+CAP_FSETID,
+CAP_IPC_LOCK,
+CAP_IPC_OWNER,
+CAP_KILL,
+CAP_LEASE,
+CAP_LINUX_IMMUTABLE,
+CAP_MAC_ADMIN,
+CAP_MAC_OVERRIDE,
+CAP_MKNOD,
+CAP_NET_ADMIN,
+CAP_NET_BIND_SERVICE,
+CAP_NET_BROADCAST,
+CAP_NET_RAW,
+CAP_SETGID,
+CAP_SETFCAP,
+CAP_SETPCAP,
+CAP_SETUID,
+CAP_SYS_ADMIN,
+CAP_SYS_BOOT,
+ 

Re: [libvirt] [PATCH 1/2] Refactor virMutexInit virRWLockInit and virCondInit

2014-07-23 Thread Daniel P. Berrange
On Wed, Jul 23, 2014 at 07:27:24AM +0200, Martin Kletzander wrote:
 On Fri, Jul 18, 2014 at 07:30:38AM -0600, Eric Blake wrote:
 On 07/17/2014 10:49 PM, Jincheng Miao wrote:
 Implement InitInternal functions for Mutex, RWLock and Cond, these functions
 contain error report using virReportSystemErrorFull, it is controlled by
 an argument 'quite'.
 The related macros are Init and InitQuite, they call InitInternal function
 by passing 'false' or 'true' to quite argument.
 
 After your patch 2, do we really have any callers that use the quiet
 version? This seems like the sort of patch where ALL callers should be
 noisy (especially since the failure is so rare, but also means we are
 quite hosed if it happens).  What are the callers that you intend to be
 quiet, and what is the justification for them being quiet?
 
 
 I think there are few callers that error out with VIR_ERROR(), but
 most of them just return -1 with no apparent reason.  I'm not sure
 about few class initializations (virOnceInit) and driver
 initializations.  Do we want to be loud on that front as well?

Yes, global initializers must always report proper errors - these
are captured  reported on every call even though the initializer
is only run once.  So yes, I think everything should be noisy in
this case - there's no legitimate reason with these fnuctions why
you would want to be quiet and/or ignore failure.

Regards,
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/8] Speed up waiting for the session daemon

2014-07-23 Thread Martin Kletzander

On Tue, Jul 22, 2014 at 05:11:23PM +0200, Martin Kletzander wrote:

On Tue, Jul 22, 2014 at 01:36:56PM +0100, Daniel P. Berrange wrote:

On Wed, Jul 16, 2014 at 08:29:54PM +0200, Martin Kletzander wrote:

This is complete rework of:

http://www.redhat.com/archives/libvir-list/2013-April/msg01351.html

where Daniel suggested we use systemd-like passing of socket fd in
combination with the LISTEN_FDS environment variable:

http://www.redhat.com/archives/libvir-list/2013-April/msg01356.html

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369


Obviously that bug is focused on starting of the session daemon,
but the code we're adding here should work with systemd. Have you
tested that this actually allows for systemd activation of the
privileged libvirtd. If we're adding this FD passing, I think we
really ought to make sure we support this, so we don't have to
revisit it later. Should add a libvirtd.socket unit file too
so we have systemd activation by default for libvirtd.



Obviously I haven't.  I just wanted to get rid of that silly, silly
bug.



And now I tried and ... it Just Works(TM).  I'll add a configuration
comment and libvirtd.socket.in file in the patch and send a v3.  Let
me know what do you think of those permission settings there.


NB, we stil need to enable the daemon by default anyway since
libvirtd needs todo autostart of VMs, but having the socket
activation too avoids some race conditions with startup.



Yes, that's one of the reasons why I think it will create more
confusion than races it will resolve.

I'll _try_ to work this in, but how would you suggest to set up the
initial permissions?  Anything the user will change in libvirtd.conf
he will also have to change in the libvirt.socket file, because
someone might use the filesystem-level permission checking for
isolating some users (or anything else) because we don't want to break
that.



Regards,
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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 0/8] Speed up waiting for the session daemon

2014-07-23 Thread Daniel P. Berrange
On Wed, Jul 23, 2014 at 10:49:33AM +0200, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 05:11:23PM +0200, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 01:36:56PM +0100, Daniel P. Berrange wrote:
 On Wed, Jul 16, 2014 at 08:29:54PM +0200, Martin Kletzander wrote:
 This is complete rework of:
 
 http://www.redhat.com/archives/libvir-list/2013-April/msg01351.html
 
 where Daniel suggested we use systemd-like passing of socket fd in
 combination with the LISTEN_FDS environment variable:
 
 http://www.redhat.com/archives/libvir-list/2013-April/msg01356.html
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
 
 Obviously that bug is focused on starting of the session daemon,
 but the code we're adding here should work with systemd. Have you
 tested that this actually allows for systemd activation of the
 privileged libvirtd. If we're adding this FD passing, I think we
 really ought to make sure we support this, so we don't have to
 revisit it later. Should add a libvirtd.socket unit file too
 so we have systemd activation by default for libvirtd.
 
 
 Obviously I haven't.  I just wanted to get rid of that silly, silly
 bug.
 
 
 And now I tried and ... it Just Works(TM).  I'll add a configuration
 comment and libvirtd.socket.in file in the patch and send a v3.  Let
 me know what do you think of those permission settings there.

That's great !

Regards,
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 8/8] rpc: pass listen FD to the daemon being started

2014-07-23 Thread Daniel P. Berrange
On Wed, Jul 16, 2014 at 08:30:02PM +0200, Martin Kletzander wrote:
 - retry:
 -if (connect(fd, remoteAddr.data.sa, remoteAddr.len)  0) {
 -if ((errno == ECONNREFUSED ||
 - errno == ENOENT) 
 -spawnDaemon  retries  20) {
 -VIR_DEBUG(Connection refused for %s, trying to spawn %s,
 -  path, binary);
 -if (retries == 0 
 -virNetSocketForkDaemon(binary)  0)
 -goto error;
 +if (spawnDaemon) {
 +if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0))  0) {
 +virReportSystemError(errno, %s, _(Failed to create socket));
 +goto error;
 +}
 
 -retries++;
 -usleep(1000 * 100 * retries);
 -goto retry;
 +/*
 + * We cannot do the umask() trick here because that's not
 + * thread-safe.  fchmod(), however, is not guaranteed to work on
 + * some BSD favours, but *should* work on Linux before the socket
 + * is bound.  POSIX says the behaviour of fchmod() called on
 + * socket is unspecified, though.
 + */

IIUC, the entire concept of UNIX domain socket permissions is entirely
unspecified not just fchmod().

We could however use umask() here if you use a virFork() to start a
single threaded process. Set umask, create the socket, and then exit,
allowing the parent to continue.

 +if (fchmod(passfd, 0700)  0) {
 +virReportSystemError(errno, %s,
 + _(Failed to change permissions on 
 socket));
 +goto error;
  }
 

Regards,
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] [PATCHv2 0/2] Tri-state bool enum cleanups

2014-07-23 Thread Ján Tomko
On 07/16/2014 10:34 AM, Ján Tomko wrote:
 v2: better names
 move to virutil and use them in network_conf and device_conf too
 
 Ján Tomko (2):
   Introduce virTristateBool enum type
   Introduce virTristateSwitch enum
 
  src/conf/device_conf.c  |   8 +-
  src/conf/device_conf.h  |  10 ---
  src/conf/domain_conf.c  | 202 
 +++-
  src/conf/domain_conf.h  | 116 ++---
  src/conf/network_conf.c |  11 +--
  src/conf/network_conf.h |  15 +---
  src/libvirt_private.syms|  28 +-
  src/libxl/libxl_conf.c  |   6 +-
  src/lxc/lxc_container.c |   4 +-
  src/lxc/lxc_native.c|   2 +-
  src/network/bridge_driver.c |   3 +-
  src/qemu/qemu_command.c |  90 ++--
  src/qemu/qemu_driver.c  |   4 +-
  src/qemu/qemu_process.c |   2 +-
  src/util/virutil.c  |  11 +++
  src/util/virutil.h  |  21 +
  src/vbox/vbox_tmpl.c|  22 ++---
  src/xenxs/xen_sxpr.c|  20 ++---
  src/xenxs/xen_xm.c  |  20 ++---
  19 files changed, 200 insertions(+), 395 deletions(-)
 

I've adjusted the comments I missed, added missing BIOSUseSerial to the commit
message, added a comment about the silent yes as default to
virNetworkDNSDefFormat, resolved the conflicts when rebasing against master
and pushed the series.

Thanks for the reviews!

Jan



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-tck] docs: typo fixes

2014-07-23 Thread Roman Bogorodskiy
  Roman Bogorodskiy wrote:

 Fix a few typos in docs/intro.pod.
 ---
  docs/intro.pod | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)
 

ping?

Roman Bogorodskiy

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


Re: [libvirt] [PATCHv4 1/2] lxc: allow to keep or drop capabilities

2014-07-23 Thread Ján Tomko
On 07/18/2014 10:02 AM, Cédric Bosdonnat wrote:
 Added capabilities in the features section of LXC domains
 configuration. This section can contain elements named after the
 capabilities like:
 
   mknod state=on/, keep CAP_MKNOD capability
   sys_chroot state=off/ drop CAP_SYS_CHROOT capability
 
 Users can restrict or give more capabilities than the default using
 this mechanism.
 ---
  docs/drvlxc.html.in |  47 +
  docs/schemas/domaincommon.rng   | 207 
  src/conf/domain_conf.c  | 126 -
  src/conf/domain_conf.h  |  56 ++
  src/libvirt_private.syms|   3 +
  src/lxc/lxc_cgroup.c|   8 +
  src/lxc/lxc_container.c | 241 
 ++--
  src/util/vircgroup.c|  57 +-
  src/util/vircgroup.h|   2 +
  tests/domainschemadata/domain-caps-features.xml |  28 +++
  10 files changed, 755 insertions(+), 20 deletions(-)
  create mode 100644 tests/domainschemadata/domain-caps-features.xml
 

 @@ -11847,6 +11892,22 @@ virDomainDefParseXML(xmlDocPtr xml,
  def-features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
  break;
  
 +case VIR_DOMAIN_FEATURE_CAPABILITIES:
 +node = ctxt-node;
 +ctxt-node = nodes[i];
 +if ((tmp = virXPathString(string(./@policy), ctxt))) {
 +if ((def-features[val] = 
 virDomainCapabilitiesPolicyTypeFromString(tmp)) == -1) {

def-features is described as being of type 'enum virTristateSwitch' (was
virDomainFeatureState before I pushed the enum cleanup), but you're treating
it as 'virDomainCapabilitesPolicy' here.

Could you either
1) switch this to virTristateSwitch, using policy='on' / policy='off' instead
of allow/deny
2) document that a different enum is used for this feature in domain_conf.h
3) put the policy in a separate variable

Thanks,

Jan



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-tck] docs: typo fixes

2014-07-23 Thread Ján Tomko
On 06/27/2014 05:59 PM, Roman Bogorodskiy wrote:
 Fix a few typos in docs/intro.pod.
 ---
  docs/intro.pod | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

ACK

Jan




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] metadata: track title edits across libvirtd restart

2014-07-23 Thread Eric Blake
On 07/22/2014 11:09 PM, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 01:11:03PM -0600, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1122205

 Although the edits were changing in-memory XML, it was not flushed
 to disk; so unless some other action changes XML, a libvirtd restart
 would lose the changed information.

 
 And there are more places like that.  Take a
 qemuDomainSetNumaParameters() for example.

That's what I was afraid of - that I'd be stuck with auditing other uses
all because I stumbled on this one while testing a similar fix to
blockcopy. :/

  Would it be possible (as
 in not devastatingly hard to create a syntax-check rule for
 *_driver.c files (only those that are applicable, mostly stateful)
 that would check if virCheckFlags is called with
 VIR_DOMAIN_AFFECT_LIVE and if yes, then that function would have to
 call virDomainSaveStatus() as well?

Via a .pl script maybe, but even that is hard, because sometimes (as in
this patch) the call to virDomainSaveStatus() is delegated to a helper
function in another file than where the API call checks flags.  It's not
just AFFECT_LIVE, but any time where we modify a virDomainDefPtr
(whether live or config), then those modifications have to be written
back to disk before ending the API.

 
 [...]
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4b00280..6ed6155 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 [...]
 @@ -19753,9 +19757,12 @@ virDomainObjSetMetadata(virDomainObjPtr vm,
 persistentDef)  0)
 return -1;

 -if (flags  VIR_DOMAIN_AFFECT_LIVE)
 +if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 if (virDomainDefSetMetadata(vm-def, type, metadata, key, uri)
  0)
 return -1;
 
 One empty line right here would perfectly go with the rest of the
 function ;)
 
 ACK with or without this change,
 
 Martin

Thanks, will push shortly.

-- 
Eric Blake   eblake 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/2] util: Introduce flags field for macvtap creation

2014-07-23 Thread Martin Kletzander

On Tue, Jul 01, 2014 at 02:00:56PM -0400, Matthew Rosato wrote:

Currently, there is one flag passed in during macvtap creation
(withTap) -- Let's convert this field to an unsigned int flag
field for future expansion.

Signed-off-by: Matthew Rosato mjros...@linux.vnet.ibm.com
---
src/lxc/lxc_process.c   |2 +-
src/qemu/qemu_command.c |3 ++-
src/util/virnetdevmacvlan.c |   24 +++-
src/util/virnetdevmacvlan.h |8 +++-
4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index cb85b74..bfbecbf 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -790,6 +790,7 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char 
*ifname,
 * @macaddress: The MAC address for the macvtap device
 * @linkdev: The interface name of the NIC to connect to the external bridge
 * @mode: int describing the mode for 'bridge', 'vepa', 'private' or 'passthru'.
+ * @flags: OR of virNetDevMacVLanCreateFlags.
 * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it
 * @vmuuid: The UUID of the VM the macvtap belongs to
 * @virtPortProfile: pointer to object holding the virtual port profile data
@@ -797,14 +798,15 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char 
*ifname,
 * interface will be stored into if everything succeeded. It is up
 * to the caller to free the string.
 *
- * Returns file descriptor of the tap device in case of success with @withTap,
- * otherwise returns 0; returns -1 on error.
+ * Returns file descriptor of the tap device in case of success with
+ * flags  VIR_NETDEV_MACVLAN_CREATE_WITH_TAP, otherwise returns 0; returns


s/flags/@flags/


+ * -1 on error.
 */
int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
   const virMacAddr *macaddress,
   const char *linkdev,
   virNetDevMacVLanMode mode,
-   bool withTap,
+   unsigned int flags,
   int vnet_hdr,
   const unsigned char *vmuuid,
   virNetDevVPortProfilePtr 
virtPortProfile,


Having @flags as some middle parameter feels itchy, could you move it
to the end?

[...]

@@ -813,9 +815,12 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
*tgifname,
   char *stateDir,
   virNetDevBandwidthPtr bandwidth)
{
-const char *type = withTap ? macvtap : macvlan;
-const char *prefix = withTap ? MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
-const char *pattern = withTap ? MACVTAP_NAME_PATTERN : 
MACVLAN_NAME_PATTERN;
+const char *type = (flags  VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+macvtap : macvlan;
+const char *prefix = (flags  VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
+const char *pattern = (flags  VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;


Since these are now multiline anyway, it would be nice to have them
set in an if for more readability.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 8/8] rpc: pass listen FD to the daemon being started

2014-07-23 Thread Eric Blake
On 07/23/2014 03:25 AM, Daniel P. Berrange wrote:

 We could however use umask() here if you use a virFork() to start a
 single threaded process. Set umask, create the socket, and then exit,
 allowing the parent to continue.
 

Similar to what we already do in virfile.c for virFileAccessibleAs,
virFileOpenAs, and virDirCreate.


-- 
Eric Blake   eblake 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] metadata: track title edits across libvirtd restart

2014-07-23 Thread Martin Kletzander

On Wed, Jul 23, 2014 at 06:46:24AM -0600, Eric Blake wrote:

On 07/22/2014 11:09 PM, Martin Kletzander wrote:

On Tue, Jul 22, 2014 at 01:11:03PM -0600, Eric Blake wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1122205

Although the edits were changing in-memory XML, it was not flushed
to disk; so unless some other action changes XML, a libvirtd restart
would lose the changed information.



And there are more places like that.  Take a
qemuDomainSetNumaParameters() for example.


That's what I was afraid of - that I'd be stuck with auditing other uses
all because I stumbled on this one while testing a similar fix to
blockcopy. :/


 Would it be possible (as
in not devastatingly hard to create a syntax-check rule for
*_driver.c files (only those that are applicable, mostly stateful)
that would check if virCheckFlags is called with
VIR_DOMAIN_AFFECT_LIVE and if yes, then that function would have to
call virDomainSaveStatus() as well?


Via a .pl script maybe, but even that is hard, because sometimes (as in
this patch) the call to virDomainSaveStatus() is delegated to a helper
function in another file than where the API call checks flags.  It's not
just AFFECT_LIVE, but any time where we modify a virDomainDefPtr
(whether live or config), then those modifications have to be written
back to disk before ending the API.



Or maybe CIL or something similar can be used for that.  Having
syntax-check work with the intermediate language would have both pros
and cons, I guess.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] nodedev: fix pci express memory leak

2014-07-23 Thread Eric Blake
On 07/22/2014 11:20 PM, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 10:42:29PM -0600, Eric Blake wrote:
 Leak introduced in commit 16ebf10f (v1.2.6), detected by valgrind:

 ==9816== 216 (96 direct, 120 indirect) bytes in 6 blocks are
 definitely lost in loss record 665 of 821
 ==9816==at 0x4A081D4: calloc (in
 /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 ==9816==by 0x50836FB: virAlloc (viralloc.c:144)
 ==9816==by 0x1DBDBE27: udevProcessPCI (node_device_udev.c:546)
 ==9816==by 0x1DBDD79D: udevGetDeviceDetails (node_device_udev.c:1293)

 While at it, fix switch statements to be a bit more vocal if we forget
 some cases when adding new devices.

 * src/conf/node_device_conf.c (virNodeDeviceDefFormat)
 (virNodeDevCapsDefParseXML): Drop default case.
 (virNodeDevCapsDefFree): Likewise, and clear pci_express under pci
 case.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

 I could be persuaded to split this into two patches.

 
 That would be nice, now that you've mentioned it :)

Will do, in v2.


 VIR_FREE(data-pci_dev.iommuGroupDevices);
 +VIR_FREE(data-pci_dev.pci_express);
 
 There should be data-pci_dev.pci_express-link_{sta,cap} free()'d too.
 
 ACK with that changed.

Then node_device_udev.c has the same bug in udevProcessPCI.  All the
more reason for me to respin.

 
 Martin

-- 
Eric Blake   eblake 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 v2 8/8] rpc: pass listen FD to the daemon being started

2014-07-23 Thread Martin Kletzander

On Wed, Jul 23, 2014 at 07:01:06AM -0600, Eric Blake wrote:

On 07/23/2014 03:25 AM, Daniel P. Berrange wrote:


We could however use umask() here if you use a virFork() to start a
single threaded process. Set umask, create the socket, and then exit,
allowing the parent to continue.



Similar to what we already do in virfile.c for virFileAccessibleAs,
virFileOpenAs, and virDirCreate.



Oh, I had the feeling that I saw such scenario somewhere.  And now
that I have v3 of the patches working, I won't be able to get some
kind of abstracting this out of my head.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] spec: Enable sanlock on qemu_kvm_arches for RHEL

2014-07-23 Thread Eric Blake
On 07/23/2014 01:07 AM, Jiri Denemark wrote:
 Signed-off-by: Jiri Denemark jdene...@redhat.com
 ---
  libvirt.spec.in | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index 76e57aa..c035c68 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -260,12 +260,12 @@
  %endif
  
  # Enable sanlock library for lock management with QEMU
 -# Sanlock is available only on x86_64 for RHEL
 +# Sanlock is available only on arches where kvm is available for RHEL
  %if 0%{?fedora} = 16
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  %endif
 -%if 0%{?rhel} = 6
 -%ifarch x86_64
 +%if 0%{?rhel}
 +%ifarch %{qemu_kvm_arches}
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}

sanlock doesn't work on RHEL 5.  I agree with the %ifarch line change,
but think the %if 0%{?rhel} line should remain untouched.

ACK with that fixed.

-- 
Eric Blake   eblake 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/2] network: Bring netdevs online later

2014-07-23 Thread Martin Kletzander

On Tue, Jul 01, 2014 at 02:00:57PM -0400, Matthew Rosato wrote:

Defer MAC registration until net devices are actually going
to be used by the guest.  This patch does so by setting the
devices online just before starting guest CPUs.



Does this have some upside/downside?  Are you trying to fix some
problem?  It would be nice to describe it in the commit message, so I
know what to focus on or why it's needed.  Depending on the answer
there might be a way how to unit-test it.


Signed-off-by: Matthew Rosato mjros...@linux.vnet.ibm.com
---
src/Makefile.am |1 +
src/conf/domain_conf.h  |2 ++
src/lxc/lxc_process.c   |3 +-
src/qemu/qemu_command.c |6 +++-
src/qemu/qemu_hotplug.c |7 +
src/qemu/qemu_interface.c   |   65 +++
src/qemu/qemu_interface.h   |   33 ++
src/qemu/qemu_process.c |4 +++
src/util/virnetdevmacvlan.c |8 --
src/util/virnetdevmacvlan.h |2 ++
10 files changed, 126 insertions(+), 5 deletions(-)
create mode 100644 src/qemu/qemu_interface.c
create mode 100644 src/qemu/qemu_interface.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 35720be..e3d2e36 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -687,6 +687,7 @@ QEMU_DRIVER_SOURCES =   
\
qemu/qemu_domain.c qemu/qemu_domain.h   \
qemu/qemu_cgroup.c qemu/qemu_cgroup.h   \
qemu/qemu_hostdev.c qemu/qemu_hostdev.h \
+   qemu/qemu_interface.c qemu/qemu_interface.h \
qemu/qemu_hotplug.c qemu/qemu_hotplug.h \
qemu/qemu_hotplugpriv.h \
qemu/qemu_conf.c qemu/qemu_conf.h   \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1122eb2..8375106 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -921,6 +921,8 @@ struct _virDomainNetDef {
virNetDevBandwidthPtr bandwidth;
virNetDevVlan vlan;
int linkstate;
+/* vmOp value saved if deferring interface start */
+virNetDevVPortProfileOp vmOp;
};

/* Used for prefix of ifname of any network name generated dynamically
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index ab0c5d0..3083ed3 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -302,6 +302,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
virNetDevBandwidthPtr bw;
virNetDevVPortProfilePtr prof;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
+unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_IFUP;

/* XXX how todo bandwidth controls ?
 * Since the 'net-ifname' is about to be moved to a different
@@ -333,7 +334,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
net-ifname, net-mac,
virDomainNetGetActualDirectDev(net),
virDomainNetGetActualDirectMode(net),
-0, false, def-uuid,
+macvlan_create_flags, false, def-uuid,
virDomainNetGetActualVirtPortProfile(net),
res_ifname,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 314d8a3..a5662f5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -196,6 +196,9 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
net-ifname = res_ifname;
}

+/* Save vport profile op for later */
+net-vmOp = vmop;
+
virObjectUnref(cfg);
return rc;

@@ -294,7 +297,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
{
char *brname = NULL;
int ret = -1;
-unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+unsigned int tap_create_flags = 0;
bool template_ifname = false;
int actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -354,6 +357,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
goto cleanup;
}
} else {
+tap_create_flags |= VIR_NETDEV_TAP_CREATE_IFUP;
if (qemuCreateInBridgePortWithHelper(cfg, brname,
 net-ifname,
 tapfd, tap_create_flags)  0) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5e8aa4e..98e7764 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -49,6 +49,7 @@
#include virstoragefile.h
#include virstring.h
#include virtime.h
+#include qemu_interface.h

#define VIR_FROM_THIS VIR_FROM_QEMU

@@ -854,6 +855,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (networkAllocateActualDevice(vm-def, net)  0)
goto cleanup;

+/* Set device online immediately */
+qemuInterfaceStartDevice(net);
+
actualType = virDomainNetGetActualType(net);

if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {

[libvirt] [PATCH] docs: Point to list of valid pool target volume formats

2014-07-23 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1092886

Rather than point off to some nefarious pool-specific docs page when
describing the format field for the target pool provide a link to the
storage driver page which describes the various valid formats for each
pool type.  Also make it a bit more clear that if a valid format isn't
specified, then the type field is ignored.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatstorage.html.in | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index e264985..e25bba7 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -508,8 +508,12 @@
 or directory pools it will provide the file format type, eg cow,
 qcow, vmdk, raw. If omitted when creating a volume, the pool's
 default format will be used. The actual format is specified via
-the codetype/code attribute. Consult the pool-specific docs for
-the list of valid values. span class=sinceSince 0.4.1/span/dd
+the codetype/code attribute. Consult the
+a href=storage.htmlstorage driver page/a for the list of valid
+volume format type values for each specific pool. The
+codeformat/code will be ignored on input for pools without a
+volume format type value and the default pool format will be used.
+span class=sinceSince 0.4.1/span/dd
   dtcodepermissions/code/dt
   ddProvides information about the default permissions to use
 when creating volumes. This is currently only useful for directory
-- 
1.9.3

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


Re: [libvirt] [PATCH 2/2] network: Bring netdevs online later

2014-07-23 Thread Eric Blake
On 07/23/2014 07:49 AM, Martin Kletzander wrote:
 On Tue, Jul 01, 2014 at 02:00:57PM -0400, Matthew Rosato wrote:
 Defer MAC registration until net devices are actually going
 to be used by the guest.  This patch does so by setting the
 devices online just before starting guest CPUs.

 
 Does this have some upside/downside?  Are you trying to fix some
 problem?  It would be nice to describe it in the commit message, so I
 know what to focus on or why it's needed.  Depending on the answer
 there might be a way how to unit-test it.
 

 +++ b/src/qemu/qemu_interface.c
 @@ -0,0 +1,65 @@
 +/*
 + * qemu_interface.c: QEMU interface management
 + *
 + * Copyright (C) 2014 Red Hat, Inc.
 + * Copyright IBM Corp. 2014
 + *
 
 I don't understand this double copyright here, copy-paste mistake?

If this file is copied from a pre-existing file with double copyright,
and substantially borrows from that content rather than being fresh
material, then keeping double copyright is ideal.  If this is something
you mostly wrote yourself, and then just copied in boilerplate, then
it's simpler to just put one copyright line for yourself (in this case,
IBM); then later, if other contributors make contributions, a second
line can be added then (I tend to add Red Hat copyright whenever I touch
files, since I'm doing my work on Red Hat time).  I don't see it as a
show-stopper either way, because these days, git log is more reliable
for telling who contributed what portions of a file.

-- 
Eric Blake   eblake 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] docs: Point to list of valid pool target volume formats

2014-07-23 Thread Martin Kletzander

On Wed, Jul 23, 2014 at 09:58:02AM -0400, John Ferlan wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1092886

Rather than point off to some nefarious pool-specific docs page when
describing the format field for the target pool provide a link to the
storage driver page which describes the various valid formats for each
pool type.  Also make it a bit more clear that if a valid format isn't
specified, then the type field is ignored.

Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatstorage.html.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)



ACK,

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 3/9] rpc: set listen backlog on FDs as well as on other sockets

2014-07-23 Thread Martin Kletzander
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/locking/lock_daemon.c | 2 +-
 src/rpc/virnetserverservice.c | 5 +
 src/rpc/virnetserverservice.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index e9219d5..02d77e3 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -614,7 +614,7 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
 #if WITH_GNUTLS
  NULL,
 #endif
- false, 1)))
+ false, 0, 1)))
 return -1;

 if (virNetServerAddService(srv, svc, NULL)  0) {
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index e85889b..fea05c3 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -133,6 +133,7 @@ virNetServerServiceNewFDOrUNIX(const char *path,
 tls,
 #endif
 readonly,
+max_queued_clients,
 nrequests_client_max);
 }
 }
@@ -265,6 +266,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
 virNetTLSContextPtr tls,
 #endif
 bool readonly,
+size_t max_queued_clients,
 size_t nrequests_client_max)
 {
 virNetServerServicePtr svc;
@@ -292,6 +294,9 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
 goto error;

 for (i = 0; i  svc-nsocks; i++) {
+if (virNetSocketListen(svc-socks[i], max_queued_clients)  0)
+goto error;
+
 /* IO callback is initially disabled, until we're ready
  * to deal with incoming clients */
 if (virNetSocketAddIOCallback(svc-socks[i],
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index a1c8960..b1d6c2d 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -74,6 +74,7 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
 virNetTLSContextPtr tls,
 # endif
 bool readonly,
+size_t max_queued_clients,
 size_t nrequests_client_max);

 virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr 
object);
-- 
2.0.2

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


[libvirt] [PATCH v3 1/9] util: abstract parsing of passed FDs into virGetListenFDs()

2014-07-23 Thread Martin Kletzander
Since not only systemd can do this (we'll be doing it as well few
patches later), change 'systemd' to 'caller' and fix LISTEN_FDS to
LISTEN_PID where applicable.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/libvirt_private.syms  |  1 +
 src/locking/lock_daemon.c | 45 +++---
 src/util/virutil.c| 62 +++
 src/util/virutil.h|  2 ++
 4 files changed, 69 insertions(+), 41 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 51504d1..d883990 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2103,6 +2103,7 @@ virGetGroupID;
 virGetGroupList;
 virGetGroupName;
 virGetHostname;
+virGetListenFDs;
 virGetSelfLastChanged;
 virGetUnprivSGIOSysfsPath;
 virGetUserCacheDirectory;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 3379f29..e9219d5 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -600,50 +600,13 @@ static int
 virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
 {
 virNetServerServicePtr svc;
-const char *pidstr;
-const char *fdstr;
-unsigned long long procid;
 unsigned int nfds;

-VIR_DEBUG(Setting up networking from systemd);
-
-if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ull(pidstr, NULL, 10, procid)  0) {
-VIR_DEBUG(Malformed LISTEN_PID from systemd %s, pidstr);
-return 0;
-}
-
-if ((pid_t)procid != getpid()) {
-VIR_DEBUG(LISTEN_PID %s is not for us %llu,
-  pidstr, (unsigned long long)getpid());
-return 0;
-}
-
-if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
-VIR_DEBUG(No LISTEN_FDS from systemd);
-return 0;
-}
-
-if (virStrToLong_ui(fdstr, NULL, 10, nfds)  0) {
-VIR_DEBUG(Malformed LISTEN_FDS from systemd %s, fdstr);
-return 0;
-}
-
-if (nfds  1) {
-VIR_DEBUG(Too many (%d) file descriptors from systemd,
-  nfds);
-nfds = 1;
-}
-
-unsetenv(LISTEN_PID);
-unsetenv(LISTEN_FDS);
-
-if (nfds == 0)
+if ((nfds = virGetListenFDs()) == 0)
 return 0;
+if (nfds  1)
+VIR_DEBUG(Too many (%d) file descriptors from systemd, nfds);
+nfds = 1;

 /* Systemd passes FDs, starting immediately after stderr,
  * so the first FD we'll get is '3'. */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 50faf2b..1d897d9 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2396,3 +2396,65 @@ void virUpdateSelfLastChanged(const char *path)
 selfLastChanged = sb.st_ctime;
 }
 }
+
+/**
+ * virGetListenFDs:
+ *
+ * Parse LISTEN_PID and LISTEN_FDS passed from caller.
+ *
+ * Returns number of passed FDs.
+ */
+unsigned int
+virGetListenFDs(void)
+{
+const char *pidstr;
+const char *fdstr;
+size_t i = 0;
+unsigned long long procid;
+unsigned int nfds;
+
+VIR_DEBUG(Setting up networking from caller);
+
+if (!(pidstr = virGetEnvAllowSUID(LISTEN_PID))) {
+VIR_DEBUG(No LISTEN_PID from caller);
+return 0;
+}
+
+if (virStrToLong_ull(pidstr, NULL, 10, procid)  0) {
+VIR_DEBUG(Malformed LISTEN_PID from caller %s, pidstr);
+return 0;
+}
+
+if ((pid_t)procid != getpid()) {
+VIR_DEBUG(LISTEN_PID %s is not for us %llu,
+  pidstr, (unsigned long long)getpid());
+return 0;
+}
+
+if (!(fdstr = virGetEnvAllowSUID(LISTEN_FDS))) {
+VIR_DEBUG(No LISTEN_FDS from caller);
+return 0;
+}
+
+if (virStrToLong_ui(fdstr, NULL, 10, nfds)  0) {
+VIR_DEBUG(Malformed LISTEN_FDS from caller %s, fdstr);
+return 0;
+}
+
+unsetenv(LISTEN_PID);
+unsetenv(LISTEN_FDS);
+
+VIR_DEBUG(Got %u file descriptors, nfds);
+
+for (i = 0; i  nfds; i++) {
+int fd = STDERR_FILENO + i + 1;
+
+VIR_DEBUG(Disabling inheritance of passed FD %d, fd);
+
+if (virSetInherit(fd, false)  0) {
+VIR_WARN(Couldn't disable inheritance of passed FD %d, fd);
+}
+}
+
+return nfds;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index f93ea93..89b7923 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -232,4 +232,6 @@ typedef enum {
 VIR_ENUM_DECL(virTristateBool)
 VIR_ENUM_DECL(virTristateSwitch)

+unsigned int virGetListenFDs(void);
+
 #endif /* __VIR_UTIL_H__ */
-- 
2.0.2

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


[libvirt] [PATCH v3 2/9] remote: create virNetServerServiceNewFDOrUNIX() wrapper

2014-07-23 Thread Martin Kletzander
It's just a wrapper around NewFD and NewUNIX that selects the right
option and increments the number of used FDs.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/libvirt_remote.syms   |  1 +
 src/rpc/virnetserverservice.c | 50 ++-
 src/rpc/virnetserverservice.h | 14 +++-
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index d482a55..6b520b5 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -159,6 +159,7 @@ virNetServerServiceGetMaxRequests;
 virNetServerServiceGetPort;
 virNetServerServiceIsReadonly;
 virNetServerServiceNewFD;
+virNetServerServiceNewFDOrUNIX;
 virNetServerServiceNewPostExecRestart;
 virNetServerServiceNewTCP;
 virNetServerServiceNewUNIX;
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 320a02c..e85889b 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -1,7 +1,7 @@
 /*
  * virnetserverservice.c: generic network RPC server service
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -25,6 +25,8 @@

 #include virnetserverservice.h

+#include unistd.h
+
 #include viralloc.h
 #include virerror.h
 #include virthread.h
@@ -90,6 +92,52 @@ static void virNetServerServiceAccept(virNetSocketPtr sock,
 }


+virNetServerServicePtr
+virNetServerServiceNewFDOrUNIX(const char *path,
+   mode_t mask,
+   gid_t grp,
+   int auth,
+#if WITH_GNUTLS
+   virNetTLSContextPtr tls,
+#endif
+   bool readonly,
+   size_t max_queued_clients,
+   size_t nrequests_client_max,
+   unsigned int nfds,
+   unsigned int *cur_fd)
+{
+if (*cur_fd - STDERR_FILENO  nfds) {
+/*
+ * There are no more file descriptors to use, so we have to
+ * fallback to UNIX socket.
+ */
+return virNetServerServiceNewUNIX(path,
+  mask,
+  grp,
+  auth,
+#if WITH_GNUTLS
+  tls,
+#endif
+  readonly,
+  max_queued_clients,
+  nrequests_client_max);
+
+} else {
+/*
+ * There's still enough file descriptors.  In this case we'll
+ * use the current one and increment it afterwards.
+ */
+return virNetServerServiceNewFD(*cur_fd++,
+auth,
+#if WITH_GNUTLS
+tls,
+#endif
+readonly,
+nrequests_client_max);
+}
+}
+
+
 virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
  const char *service,
  int auth,
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index eb31abf..a1c8960 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -1,7 +1,7 @@
 /*
  * virnetserverservice.h: generic network RPC server service
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -37,6 +37,18 @@ typedef int 
(*virNetServerServiceDispatchFunc)(virNetServerServicePtr svc,
virNetSocketPtr sock,
void *opaque);

+virNetServerServicePtr virNetServerServiceNewFDOrUNIX(const char *path,
+  mode_t mask,
+  gid_t grp,
+  int auth,
+# if WITH_GNUTLS
+  virNetTLSContextPtr tls,
+# endif
+  bool readonly,
+  size_t 
max_queued_clients,
+  size_t 
nrequests_client_max,
+  unsigned int nfds,
+  unsigned int *cur_fd);
 virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
  const char *service,
   

[libvirt] [PATCH v3 0/9] Series on passing FDs to daemon

2014-07-23 Thread Martin Kletzander
This started as a fix for virsh 20s timeout of waiting for session
daemon that failed to start:

http://www.redhat.com/archives/libvir-list/2013-April/msg01351.html

Then there was a idea that we can pass some FDs around:

http://www.redhat.com/archives/libvir-list/2013-April/msg01356.html

So we did:

https://www.redhat.com/archives/libvir-list/2014-July/msg00841.html

And now we are even able to start with socket-activation with systemd;
see patch 9/9.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369


Martin Kletzander (9):
  util: abstract parsing of passed FDs into virGetListenFDs()
  remote: create virNetServerServiceNewFDOrUNIX() wrapper
  rpc: set listen backlog on FDs as well as on other sockets
  daemon: support passing FDs from the calling process
  cfg.mk: allow integers to be assigned a value computed with i|j|k
  tests: support dynamic prefixes in commandtest
  util: add virCommandPassListenFDs() function
  rpc: pass listen FD to the daemon being started
  daemon: use socket activation with systemd

 .gitignore|   1 +
 cfg.mk|   2 +-
 daemon/Makefile.am|  14 +-
 daemon/libvirtd.c |  45 ++
 daemon/libvirtd.conf  |   5 ++
 daemon/libvirtd.service.in|   5 --
 daemon/libvirtd.socket.in |   6 +++
 libvirt.spec.in   |  26 +--
 src/libvirt_private.syms  |   2 +
 src/libvirt_remote.syms   |   1 +
 src/locking/lock_daemon.c |  47 ++-
 src/rpc/virnetserverservice.c |  55 +-
 src/rpc/virnetserverservice.h |  15 +-
 src/rpc/virnetsocket.c| 102 
 src/util/vircommand.c |  99 +++
 src/util/vircommand.h |   4 +-
 src/util/virutil.c|  62 +
 src/util/virutil.h|   2 +
 tests/commanddata/test24.log  |   7 +++
 tests/commandtest.c   | 105 ++
 20 files changed, 491 insertions(+), 114 deletions(-)
 create mode 100644 daemon/libvirtd.socket.in
 create mode 100644 tests/commanddata/test24.log

--
2.0.2

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


[libvirt] [PATCH v3 6/9] tests: support dynamic prefixes in commandtest

2014-07-23 Thread Martin Kletzander
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tests/commandtest.c | 49 ++---
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/tests/commandtest.c b/tests/commandtest.c
index 7d2161c..ba823f7 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -62,7 +62,8 @@ main(void)

 #else

-static int checkoutput(const char *testname)
+static int checkoutput(const char *testname,
+   char *prefix)
 {
 int ret = -1;
 char *expectname = NULL;
@@ -86,6 +87,16 @@ static int checkoutput(const char *testname)
 goto cleanup;
 }

+if (prefix) {
+char *tmp = NULL;
+
+if (virAsprintf(tmp, %s%s, prefix, expectlog)  0)
+goto cleanup;
+
+VIR_FREE(expectlog);
+expectlog = tmp;
+}
+
 if (STRNEQ(expectlog, actuallog)) {
 virtTestDifference(stderr, expectlog, actuallog);
 goto cleanup;
@@ -173,7 +184,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)
 return -1;
 }

-if ((ret = checkoutput(test2)) != 0) {
+if ((ret = checkoutput(test2, NULL)) != 0) {
 virCommandFree(cmd);
 return ret;
 }
@@ -187,7 +198,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test2);
+return checkoutput(test2, NULL);
 }

 /*
@@ -219,7 +230,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test3);
+ret = checkoutput(test3, NULL);

  cleanup:
 virCommandFree(cmd);
@@ -261,7 +272,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED)
 while (kill(pid, 0) != -1)
 usleep(100*1000);

-ret = checkoutput(test4);
+ret = checkoutput(test4, NULL);

  cleanup:
 virCommandFree(cmd);
@@ -291,7 +302,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test5);
+return checkoutput(test5, NULL);
 }


@@ -315,7 +326,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test6);
+return checkoutput(test6, NULL);
 }


@@ -340,7 +351,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test7);
+return checkoutput(test7, NULL);
 }

 /*
@@ -365,7 +376,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test8);
+return checkoutput(test8, NULL);
 }


@@ -403,7 +414,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test9);
+return checkoutput(test9, NULL);
 }


@@ -429,7 +440,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test10);
+return checkoutput(test10, NULL);
 }

 /*
@@ -453,7 +464,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test11);
+return checkoutput(test11, NULL);
 }

 /*
@@ -475,7 +486,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED)

 virCommandFree(cmd);

-return checkoutput(test12);
+return checkoutput(test12, NULL);
 }

 /*
@@ -510,7 +521,7 @@ static int test13(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test13);
+ret = checkoutput(test13, NULL);

  cleanup:
 virCommandFree(cmd);
@@ -582,7 +593,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test14);
+ret = checkoutput(test14, NULL);

  cleanup:
 virCommandFree(cmd);
@@ -613,7 +624,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test15);
+ret = checkoutput(test15, NULL);

  cleanup:
 VIR_FREE(cwd);
@@ -659,7 +670,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test16);
+ret = checkoutput(test16, NULL);

  cleanup:
 virCommandFree(cmd);
@@ -841,7 +852,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test20);
+ret = checkoutput(test20, NULL);
  cleanup:
 virCommandFree(cmd);
 VIR_FREE(buf);
@@ -900,7 +911,7 @@ static int test21(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }

-ret = checkoutput(test21);
+ret = checkoutput(test21, NULL);
  cleanup:
 VIR_FREE(outbuf);
 VIR_FREE(errbuf);
-- 
2.0.2

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


[libvirt] [PATCH v3 4/9] daemon: support passing FDs from the calling process

2014-07-23 Thread Martin Kletzander
First FD is the RW unix socket to listen on, second one (if
applicable) is the RO unix socket.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 daemon/libvirtd.c | 45 +++--
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 4c926b3..15f0ce2 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -56,6 +56,7 @@
 #include virstring.h
 #include locking/lock_manager.h
 #include viraccessmanager.h
+#include virutil.h

 #ifdef WITH_DRIVER_MODULES
 # include driver.h
@@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv,
 int unix_sock_ro_mask = 0;
 int unix_sock_rw_mask = 0;

+unsigned int cur_fd = STDERR_FILENO + 1;
+unsigned int nfds = virGetListenFDs();
+
 if (config-unix_sock_group) {
 if (virGetGroupID(config-unix_sock_group, unix_sock_gid)  0)
 return -1;
 }

+if (nfds  nfds  ((int)!!sock_path + (int)!!sock_path_ro)) {
+VIR_ERROR(_(Too many (%u) FDs passed from caller), nfds);
+return -1;
+}
+
 if (virStrToLong_i(config-unix_sock_ro_perms, NULL, 8, 
unix_sock_ro_mask) != 0) {
 VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_ro_perms);
 goto error;
@@ -491,30 +500,30 @@ static int daemonSetupNetworking(virNetServerPtr srv,
 goto error;
 }

-VIR_DEBUG(Registering unix socket %s, sock_path);
-if (!(svc = virNetServerServiceNewUNIX(sock_path,
-   unix_sock_rw_mask,
-   unix_sock_gid,
-   config-auth_unix_rw,
+if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
+   unix_sock_rw_mask,
+   unix_sock_gid,
+   config-auth_unix_rw,
 #if WITH_GNUTLS
-   NULL,
+   NULL,
 #endif
-   false,
-   config-max_queued_clients,
-   config-max_client_requests)))
+   false,
+   config-max_queued_clients,
+   config-max_client_requests,
+   nfds, cur_fd)))
 goto error;
 if (sock_path_ro) {
-VIR_DEBUG(Registering unix socket %s, sock_path_ro);
-if (!(svcRO = virNetServerServiceNewUNIX(sock_path_ro,
- unix_sock_ro_mask,
- unix_sock_gid,
- config-auth_unix_ro,
+if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
+ unix_sock_ro_mask,
+ unix_sock_gid,
+ config-auth_unix_ro,
 #if WITH_GNUTLS
- NULL,
+ NULL,
 #endif
- true,
- config-max_queued_clients,
- config-max_client_requests)))
+ true,
+ 
config-max_queued_clients,
+ 
config-max_client_requests,
+ nfds, cur_fd)))
 goto error;
 }

-- 
2.0.2

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


[libvirt] [PATCH v3 5/9] cfg.mk: allow integers to be assigned a value computed with i|j|k

2014-07-23 Thread Martin Kletzander
Even line like this:

int asdf = i - somevar;

was matched by the regex, so this patch adds '=' to the list of
characters that break the regexp.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 cfg.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cfg.mk b/cfg.mk
index 0559edd..f647aed 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -568,7 +568,7 @@ sc_avoid_attribute_unused_in_header:
  $(_sc_search_regexp)

 sc_prohibit_int_ijk:
-   @prohibit='\(int|unsigned) ([^(]* )*(i|j|k)\(\s|,|;)' \
+   @prohibit='\(int|unsigned) ([^(=]* )*(i|j|k)\(\s|,|;)'\
halt='use size_t, not int/unsigned int for loop vars i, j, k'   \
  $(_sc_search_regexp)

-- 
2.0.2

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


[libvirt] [PATCH v3 7/9] util: add virCommandPassListenFDs() function

2014-07-23 Thread Martin Kletzander
That sets a new flag, but that flag does mean the child will get
LISTEN_FDS and LISTEN_PID environment variables properly set and
passed FDs reordered so that it corresponds with LISTEN_FDS (they must
start right after STDERR_FILENO).

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/libvirt_private.syms |  1 +
 src/util/vircommand.c| 99 
 src/util/vircommand.h|  4 +-
 tests/commanddata/test24.log |  7 
 tests/commandtest.c  | 56 +
 5 files changed, 166 insertions(+), 1 deletion(-)
 create mode 100644 tests/commanddata/test24.log

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d883990..4d13ab4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1144,6 +1144,7 @@ virCommandNewArgList;
 virCommandNewArgs;
 virCommandNonblockingFDs;
 virCommandPassFD;
+virCommandPassListenFDs;
 virCommandRawStatus;
 virCommandRequireHandshake;
 virCommandRun;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e775ba6..3b3e6f5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -66,6 +66,7 @@ enum {
 VIR_EXEC_CLEAR_CAPS = (1  2),
 VIR_EXEC_RUN_SYNC   = (1  3),
 VIR_EXEC_ASYNC_IO   = (1  4),
+VIR_EXEC_LISTEN_FDS = (1  5),
 };

 typedef struct _virCommandFD virCommandFD;
@@ -200,6 +201,78 @@ virCommandFDSet(virCommandPtr cmd,
 return 0;
 }

+static void
+virCommandReorderFDs(virCommandPtr cmd)
+{
+int maxfd = 0;
+int openmax = 0;
+size_t i = 0;
+
+if (!cmd || cmd-has_error || !cmd-npassfd)
+return;
+
+for (i = 0; i  cmd-npassfd; i++)
+maxfd = MAX(cmd-passfd[i].fd, maxfd);
+
+openmax = sysconf(_SC_OPEN_MAX);
+if (openmax  0 ||
+maxfd + cmd-npassfd  openmax)
+goto error;
+
+/*
+ * Simple two-pass sort, nothing fancy.  This is not designed for
+ * anything else than passing around 2 FDs into the child.
+ *
+ * So first dup2() them somewhere else.
+ */
+for (i = 0; i  cmd-npassfd; i++) {
+int newfd = maxfd + i + 1;
+int oldfd = cmd-passfd[i].fd;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before 
+   passing it to the child),
+ oldfd);
+goto error;
+}
+VIR_FORCE_CLOSE(cmd-passfd[i].fd);
+}
+
+VIR_DEBUG(First reorder pass done);
+
+/*
+ * And then dup2() them in orderly manner.
+ */
+for (i = 0; i  cmd-npassfd; i++) {
+int newfd = STDERR_FILENO + i + 1;
+int oldfd = maxfd + i + 1;
+if (dup2(oldfd, newfd) != newfd) {
+virReportSystemError(errno,
+ _(Cannot dup2() fd %d before 
+   passing it to the child),
+ oldfd);
+goto error;
+}
+if (virSetInherit(newfd, true)  0) {
+virReportSystemError(errno,
+ _(Cannot set O_CLOEXEC on fd %d before 
+   passing it to the child),
+ newfd);
+goto error;
+}
+VIR_FORCE_CLOSE(oldfd);
+cmd-passfd[i].fd = newfd;
+}
+
+VIR_DEBUG(Second reorder pass done);
+
+return;
+
+ error:
+cmd-has_error = -1;
+return;
+}
+
 #ifndef WIN32

 /**
@@ -678,6 +751,15 @@ virExec(virCommandPtr cmd)
 goto fork_error;
 }

+if (cmd-flags  VIR_EXEC_LISTEN_FDS) {
+virCommandReorderFDs(cmd);
+virCommandAddEnvFormat(cmd, LISTEN_PID=%u, getpid());
+virCommandAddEnvFormat(cmd, LISTEN_FDS=%zu, cmd-npassfd);
+
+if (cmd-has_error)
+goto fork_error;
+}
+
 /* Close logging again to ensure no FDs leak to child */
 virLogReset();

@@ -919,6 +1001,23 @@ virCommandPassFD(virCommandPtr cmd, int fd, unsigned int 
flags)
 }

 /**
+ * virCommandPassListenFDs:
+ * @cmd: the command to modify
+ *
+ * Pass LISTEN_FDS and LISTEN_PID environment variables into the
+ * child.  LISTEN_PID has the value of the child's PID and LISTEN_FDS
+ * is a number of passed file descriptors starting from 3.
+ */
+void
+virCommandPassListenFDs(virCommandPtr cmd)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-flags |= VIR_EXEC_LISTEN_FDS;
+}
+
+/**
  * virCommandSetPidFile:
  * @cmd: the command to modify
  * @pidfile: filename to use
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 8cdb31c..d3b286d 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -1,7 +1,7 @@
 /*
  * vircommand.h: Child command execution
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General 

[libvirt] [PATCH v3 9/9] daemon: use socket activation with systemd

2014-07-23 Thread Martin Kletzander
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 .gitignore |  1 +
 daemon/Makefile.am | 14 --
 daemon/libvirtd.conf   |  5 +
 daemon/libvirtd.service.in |  5 -
 daemon/libvirtd.socket.in  |  6 ++
 libvirt.spec.in| 26 +-
 6 files changed, 45 insertions(+), 12 deletions(-)
 create mode 100644 daemon/libvirtd.socket.in

diff --git a/.gitignore b/.gitignore
index 90fee91..9776ea1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,6 +60,7 @@
 /daemon/libvirtd.pod
 /daemon/libvirtd.policy
 /daemon/libvirtd.service
+/daemon/libvirtd.socket
 /daemon/test_libvirtd.aug
 /docs/aclperms.htmlinc
 /docs/apibuild.py.stamp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 00221ab..70b7655 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -55,6 +55,7 @@ EXTRA_DIST =  \
libvirtd.policy.in  \
libvirtd.sasl   \
libvirtd.service.in \
+   libvirtd.socket.in  \
libvirtd.sysconf\
libvirtd.sysctl \
libvirtd.aug\
@@ -388,15 +389,18 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART
 if LIBVIRT_INIT_SCRIPT_SYSTEMD

 SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
-BUILT_SOURCES += libvirtd.service
+BUILT_SOURCES += libvirtd.service libvirtd.socket

-install-init-systemd: install-sysconfig libvirtd.service
+install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket
$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
$(INSTALL_DATA) libvirtd.service \
  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+   $(INSTALL_DATA) libvirtd.socket \
+ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket

 uninstall-init-systemd: uninstall-sysconfig
rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
+   rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
 else ! LIBVIRT_INIT_SCRIPT_SYSTEMD
 install-init-systemd:
@@ -420,6 +424,12 @@ libvirtd.service: libvirtd.service.in 
$(top_builddir)/config.status
 $  $@-t   \
mv $@-t $@

+libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status
+   $(AM_V_GEN)sed  \
+   -e 's|[@]runstatedir[@]|$(runstatedir)|g'   \
+$  $@-t   \
+   mv $@-t $@
+

 check-local: check-augeas

diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index e5856d4..b644e81 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -77,6 +77,11 @@
 # UNIX socket access controls
 #

+# Beware that if you are changing *any* of these options, and you use
+# socket activation with systemd, you need to adjust the settings in
+# the libvirtd.socket file as well since it could impose a security
+# risk if you rely on file permission checking only.
+
 # Set the UNIX domain socket group ownership. This can be used to
 # allow a 'trusted' set of users access to management capabilities
 # without becoming root.
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 086da36..1759ac8 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -1,8 +1,3 @@
-# NB we don't use socket activation. When libvirtd starts it will
-# spawn any virtual machines registered for autostart. We want this
-# to occur on every boot, regardless of whether any client connects
-# to a socket. Thus socket activation doesn't have any benefit
-
 [Unit]
 Description=Virtualization daemon
 Before=libvirt-guests.service
diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in
new file mode 100644
index 000..86cc3f4
--- /dev/null
+++ b/daemon/libvirtd.socket.in
@@ -0,0 +1,6 @@
+[Socket]
+ListenStream=@runstatedir@/libvirt/libvirt-sock
+ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
+SocketMode=0777
+SocketUser=root
+SocketGroup=root
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 76e57aa..272f381 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1669,11 +1669,14 @@ done

 %if %{with_systemd}
 %if %{with_systemd_macros}
-%systemd_post virtlockd.socket libvirtd.service
+%systemd_post virtlockd.socket libvirtd.service libvirtd.socket
 %else
 if [ $1 -eq 1 ] ; then
 # Initial installation
-/bin/systemctl enable virtlockd.socket libvirtd.service /dev/null 21 || 
:
+/bin/systemctl enable \
+virtlockd.socket \
+libvirtd.service \
+libvirtd.socket /dev/null 21 || :
 fi
 %endif
 %else
@@ -1694,12 +1697,24 @@ fi
 %preun daemon
 %if %{with_systemd}
 %if %{with_systemd_macros}
-%systemd_preun libvirtd.service virtlockd.socket 

[libvirt] [PATCH v3 8/9] rpc: pass listen FD to the daemon being started

2014-07-23 Thread Martin Kletzander
This eliminates the need for active waiting.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetsocket.c | 102 -
 1 file changed, 83 insertions(+), 19 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index a94b2bc..46be541 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1,7 +1,7 @@
 /*
  * virnetsocket.c: generic network socket handling
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -121,7 +121,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket)


 #ifndef WIN32
-static int virNetSocketForkDaemon(const char *binary)
+static int virNetSocketForkDaemon(const char *binary, int passfd)
 {
 int ret;
 virCommandPtr cmd = virCommandNewArgList(binary,
@@ -134,6 +134,10 @@ static int virNetSocketForkDaemon(const char *binary)
 virCommandAddEnvPassBlockSUID(cmd, XDG_RUNTIME_DIR, NULL);
 virCommandClearCaps(cmd);
 virCommandDaemonize(cmd);
+if (passfd) {
+virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+virCommandPassListenFDs(cmd);
+}
 ret = virCommandRun(cmd, NULL);
 virCommandFree(cmd);
 return ret;
@@ -540,10 +544,11 @@ int virNetSocketNewConnectUNIX(const char *path,
const char *binary,
virNetSocketPtr *retsock)
 {
+char *buf = NULL;
+int errfd[2] = { -1, -1 };
+int fd, passfd;
 virSocketAddr localAddr;
 virSocketAddr remoteAddr;
-int fd;
-int retries = 0;

 memset(localAddr, 0, sizeof(localAddr));
 memset(remoteAddr, 0, sizeof(remoteAddr));
@@ -569,28 +574,82 @@ int virNetSocketNewConnectUNIX(const char *path,
 if (remoteAddr.data.un.sun_path[0] == '@')
 remoteAddr.data.un.sun_path[0] = '\0';

- retry:
-if (connect(fd, remoteAddr.data.sa, remoteAddr.len)  0) {
-if ((errno == ECONNREFUSED ||
- errno == ENOENT) 
-spawnDaemon  retries  20) {
-VIR_DEBUG(Connection refused for %s, trying to spawn %s,
-  path, binary);
-if (retries == 0 
-virNetSocketForkDaemon(binary)  0)
-goto error;
+if (spawnDaemon) {
+int err = 0;
+int rv = -1;
+int status = 0;
+pid_t pid = 0;

-retries++;
-usleep(1000 * 100 * retries);
-goto retry;
+if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0))  0) {
+virReportSystemError(errno, %s, _(Failed to create socket));
+goto error;
 }

-virReportSystemError(errno,
- _(Failed to connect socket to '%s'),
+if (pipe2(errfd, O_CLOEXEC)  0) {
+virReportSystemError(errno, %s,
+ _(Cannot create pipe for child));
+goto error;
+}
+
+/*
+ * We have to fork() here, because umask() is set
+ * per-process, chmod() is racy and fchmod() has undefined
+ * behaviour on sockets according to POSIX, so it doesn't
+ * work outside Linux.
+ */
+
+if ((pid = virFork())  0)
+goto error;
+
+if (pid == 0) {
+VIR_FORCE_CLOSE(errfd[0]);
+
+umask(0077);
+rv = bind(passfd, remoteAddr.data.sa, remoteAddr.len);
+if (rv  0) {
+ignore_value(safewrite(errfd[1], errno, sizeof(int)));
+}
+VIR_FORCE_CLOSE(errfd[1]);
+_exit(rv  0 ? EXIT_FAILURE : EXIT_SUCCESS);
+}
+
+VIR_FORCE_CLOSE(errfd[1]);
+rv = virProcessWait(pid, status, false);
+ignore_value(saferead(errfd[0], err, sizeof(int)));
+VIR_FORCE_CLOSE(errfd[0]);
+
+if (rv  0 || status != EXIT_SUCCESS) {
+if (err) {
+virReportSystemError(err,
+ _(Failed to bind socket to %s 
+   in child process),
+ path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Failed to bind socket to %s 
+ in child process),
+   path);
+}
+goto error;
+}
+
+if (listen(passfd, 0)  0) {
+virReportSystemError(errno, %s,
+ _(Failed to listen on socket that's about 
+   to be passed to the daemon));
+goto error;
+}
+}
+
+if (connect(fd, remoteAddr.data.sa, remoteAddr.len)  0) {
+virReportSystemError(errno, _(Failed to connect socket to '%s'),
   

Re: [libvirt] [PATCHv4 1/2] lxc: allow to keep or drop capabilities

2014-07-23 Thread Cedric Bosdonnat
On Wed, 2014-07-23 at 13:23 +0200, Ján Tomko wrote:
 On 07/18/2014 10:02 AM, Cédric Bosdonnat wrote:
  Added capabilities in the features section of LXC domains
  configuration. This section can contain elements named after the
  capabilities like:
  
mknod state=on/, keep CAP_MKNOD capability
sys_chroot state=off/ drop CAP_SYS_CHROOT capability
  
  Users can restrict or give more capabilities than the default using
  this mechanism.
  ---
   docs/drvlxc.html.in |  47 +
   docs/schemas/domaincommon.rng   | 207 
   src/conf/domain_conf.c  | 126 -
   src/conf/domain_conf.h  |  56 ++
   src/libvirt_private.syms|   3 +
   src/lxc/lxc_cgroup.c|   8 +
   src/lxc/lxc_container.c | 241 
  ++--
   src/util/vircgroup.c|  57 +-
   src/util/vircgroup.h|   2 +
   tests/domainschemadata/domain-caps-features.xml |  28 +++
   10 files changed, 755 insertions(+), 20 deletions(-)
   create mode 100644 tests/domainschemadata/domain-caps-features.xml
  
 
  @@ -11847,6 +11892,22 @@ virDomainDefParseXML(xmlDocPtr xml,
   def-features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
   break;
   
  +case VIR_DOMAIN_FEATURE_CAPABILITIES:
  +node = ctxt-node;
  +ctxt-node = nodes[i];
  +if ((tmp = virXPathString(string(./@policy), ctxt))) {
  +if ((def-features[val] = 
  virDomainCapabilitiesPolicyTypeFromString(tmp)) == -1) {
 
 def-features is described as being of type 'enum virTristateSwitch' (was
 virDomainFeatureState before I pushed the enum cleanup), but you're treating
 it as 'virDomainCapabilitesPolicy' here.
 
 Could you either
 1) switch this to virTristateSwitch, using policy='on' / policy='off' instead
 of allow/deny
 2) document that a different enum is used for this feature in domain_conf.h

Just pushed a commit documenting that in domain_conf.h

--
Cedric

 3) put the policy in a separate variable
 
 Thanks,
 
 Jan
 


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

Re: [libvirt] [PATCH libvirt-tck] docs: typo fixes

2014-07-23 Thread Roman Bogorodskiy
  Ján Tomko wrote:

 On 06/27/2014 05:59 PM, Roman Bogorodskiy wrote:
  Fix a few typos in docs/intro.pod.
  ---
   docs/intro.pod | 10 +-
   1 file changed, 5 insertions(+), 5 deletions(-)
 
 ACK

Thanks; pushed.

Roman Bogorodskiy


pgpjSe2uKYbbn.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] spec: Enable sanlock on qemu_kvm_arches for RHEL

2014-07-23 Thread Jiri Denemark
On Wed, Jul 23, 2014 at 07:39:50 -0600, Eric Blake wrote:
 On 07/23/2014 01:07 AM, Jiri Denemark wrote:
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
   libvirt.spec.in | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
  
  diff --git a/libvirt.spec.in b/libvirt.spec.in
  index 76e57aa..c035c68 100644
  --- a/libvirt.spec.in
  +++ b/libvirt.spec.in
  @@ -260,12 +260,12 @@
   %endif
   
   # Enable sanlock library for lock management with QEMU
  -# Sanlock is available only on x86_64 for RHEL
  +# Sanlock is available only on arches where kvm is available for RHEL
   %if 0%{?fedora} = 16
   %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
   %endif
  -%if 0%{?rhel} = 6
  -%ifarch x86_64
  +%if 0%{?rhel}
  +%ifarch %{qemu_kvm_arches}
   %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 
 sanlock doesn't work on RHEL 5.  I agree with the %ifarch line change,
 but think the %if 0%{?rhel} line should remain untouched.
 
 ACK with that fixed.

Right, I fixed that and pushed.

Thanks,

Jirka

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


[libvirt] [PATCH v2 0/6] Hugepages wrt NUMA

2014-07-23 Thread Michal Privoznik
Another attempt. I've dropped the 1/7 from v1, and changed a few bits raised
during review. Although, I'm still using:

  memoryBacking
hugepages
  page size='2048' unit='KiB'/
/hugepages
  /memoryBacking

as I don't feel there's any disagreement. But if there is I can rework the
patches.

Michal Privoznik (6):
  Introduce virFileFindHugeTLBFS
  qemu: Utilize virFileFindHugeTLBFS
  virbitmap: Introduce virBitmapOverlaps
  domain: Introduce ./hugepages/page/[@size,@unit,@nodeset]
  qemu: Implement ./hugepages/page/[@size,@unit,@nodeset]
  tests: Some testing of hugepages mapping

 docs/formatdomain.html.in  |  18 +-
 docs/schemas/domaincommon.rng  |  19 +-
 src/Makefile.am|  12 +-
 src/conf/domain_conf.c | 197 +++--
 src/conf/domain_conf.h |  13 +-
 src/libvirt_private.syms   |   3 +
 src/parallels/parallels_driver.c   |   2 +-
 src/qemu/qemu.conf |   9 +-
 src/qemu/qemu_capabilities.c   |   2 +
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_command.c| 111 ++--
 src/qemu/qemu_conf.c   | 124 +++--
 src/qemu/qemu_conf.h   |   9 +-
 src/qemu/qemu_driver.c |  39 ++--
 src/qemu/qemu_process.c|  21 ++-
 src/util/virbitmap.c   |  20 +++
 src/util/virbitmap.h   |   3 +
 src/util/virfile.c | 151 +++-
 src/util/virfile.h |  12 ++
 .../qemuxml2argv-hugepages-pages.args  |  16 ++
 .../qemuxml2argv-hugepages-pages.xml   |  45 +
 .../qemuxml2argv-hugepages-pages2.args |  10 ++
 .../qemuxml2argv-hugepages-pages2.xml  |  38 
 .../qemuxml2argv-hugepages-pages3.args |   9 +
 .../qemuxml2argv-hugepages-pages3.xml  |  38 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |   2 +-
 tests/qemuxml2argvtest.c   |  18 +-
 tests/qemuxml2xmltest.c|   3 +
 tests/virbitmaptest.c  |  26 +++
 29 files changed, 878 insertions(+), 93 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml

-- 
1.8.5.5

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


[libvirt] [PATCH v2 2/6] qemu: Utilize virFileFindHugeTLBFS

2014-07-23 Thread Michal Privoznik
Use better detection of hugetlbfs mount points. Yes, there can be
multiple mount points each serving different huge page size.

Since we already have ability to override the mount point in the
qemu.conf file, this crazy backward compatibility code is brought in.
Now we allow multiple mount points, so the hugetlbfs_mount option
must take an list of strings (mount points). But previously, it was
just a string, so we must accept both types now.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---

Notes:
Yes, the libvirtd_qemu.aug change is missing here. The reason is prosaic: I
don't know how to make augeas accept both array of string and a single 
string
for a configuration variable. BTW: the security_drivers which does exactly 
what
I'm doing here, accepts only single string in the aug file too.

 src/Makefile.am  |  12 ++---
 src/qemu/qemu.conf   |   9 +++-
 src/qemu/qemu_command.c  |  20 +---
 src/qemu/qemu_conf.c | 124 ++-
 src/qemu/qemu_conf.h |   9 +++-
 src/qemu/qemu_driver.c   |  39 +++
 src/qemu/qemu_process.c  |  21 +---
 tests/qemuxml2argvtest.c |  10 ++--
 8 files changed, 183 insertions(+), 61 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 982f63d..2444d51 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1209,10 +1209,10 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
$(AM_CFLAGS)
 libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS)
 libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
-$(GNUTLS_LIBS) \
-   $(LIBNL_LIBS) \
-   $(LIBXML_LIBS) \
-   $(NULL)
+$(GNUTLS_LIBS) \
+$(LIBNL_LIBS) \
+$(LIBXML_LIBS) \
+libvirt_util.la
 libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
 
 conf_DATA += qemu/qemu.conf
@@ -1995,8 +1995,8 @@ endif WITH_DRIVER_MODULES
 BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp
 
 if WITH_QEMU
-libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo
-nodist_libvirt_driver_qemu_la_SOURCES = libvirt_qemu_probes.h
+libvirt_driver_qemu_impl_la_LIBADD += libvirt_qemu_probes.lo libvirt_probes.lo
+libvirt_driver_qemu_impl_la_SOURCES += libvirt_qemu_probes.h
 BUILT_SOURCES += libvirt_qemu_probes.h
 endif WITH_QEMU
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 18ce2a8..a079b93 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -330,7 +330,14 @@
 # unspecified here, determination of a host mount point in /proc/mounts
 # will be attempted.  Specifying an explicit mount overrides detection
 # of the same in /proc/mounts.  Setting the mount point to  will
-# disable guest hugepage backing.
+# disable guest hugepage backing. If desired, multiple mount points can
+# be specified at once, separated by comma and enclosed in square
+# brackets, for example:
+#
+# hugetlbfs_mount = [/dev/hugepages2M, /dev/hugepages1G]
+#
+# The size of huge page served by specific mount point is determined by
+# libvirt at the daemon startup.
 #
 # NB, within this mount point, guests will create memory backing files
 # in a location of $MOUNTPOINT/libvirt/qemu
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fe207a4..091447a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7333,14 +7333,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 def-mem.max_balloon = VIR_DIV_UP(def-mem.max_balloon, 1024) * 1024;
 virCommandAddArgFormat(cmd, %llu, def-mem.max_balloon / 1024);
 if (def-mem.hugepage_backed) {
-if (!cfg-hugetlbfsMount) {
+char *mem_path;
+
+if (!cfg-nhugetlbfs) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   %s, _(hugetlbfs filesystem is not mounted));
-goto error;
-}
-if (!cfg-hugepagePath) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   %s, _(hugepages are disabled by administrator 
config));
+   %s, _(hugetlbfs filesystem is not mounted 
+   or disabled by administrator config));
 goto error;
 }
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
@@ -7349,8 +7347,14 @@ qemuBuildCommandLine(virConnectPtr conn,
def-emulator);
 goto error;
 }
+
+if (!(mem_path = qemuGetDefaultHugepath(cfg-hugetlbfs,
+cfg-nhugetlbfs)))
+goto error;
+
 virCommandAddArgList(cmd, -mem-prealloc, -mem-path,
- cfg-hugepagePath, NULL);
+ mem_path, NULL);
+VIR_FREE(mem_path);
 }
 
 if (def-mem.locked  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e62bec0..bed9680 100644
--- 

[libvirt] [PATCH v2 6/6] tests: Some testing of hugepages mapping

2014-07-23 Thread Michal Privoznik
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 .../qemuxml2argv-hugepages-pages2.args | 10 ++
 .../qemuxml2argv-hugepages-pages2.xml  | 38 ++
 .../qemuxml2argv-hugepages-pages3.args |  9 +
 .../qemuxml2argv-hugepages-pages3.xml  | 38 ++
 tests/qemuxml2argvtest.c   |  4 +++
 tests/qemuxml2xmltest.c|  2 ++
 6 files changed, 101 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
new file mode 100644
index 000..9211bc6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
@@ -0,0 +1,10 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 1024 -smp 2 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=256M,id=ram-node0 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=768M,id=ram-node1 \
+-numa node,nodeid=1,cpus=1,memdev=ram-node1 \
+-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
new file mode 100644
index 000..3df870b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
@@ -0,0 +1,38 @@
+domain type='qemu'
+  nameSomeDummyHugepagesGuest/name
+  uuidef1bdff4-27f3-4e85-a807-5fb4d58463cc/uuid
+  memory unit='KiB'1048576/memory
+  currentMemory unit='KiB'1048576/currentMemory
+  memoryBacking
+hugepages
+  page size='2048' unit='KiB'/
+/hugepages
+  /memoryBacking
+  vcpu placement='static'2/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  cpu
+numa
+  cell id='0' cpus='0' memory='262144'/
+  cell id='1' cpus='1' memory='786432'/
+/numa
+  /cpu
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
new file mode 100644
index 000..27b3f8e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
@@ -0,0 +1,9 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 1024 -smp 2 \
+-object memory-backend-ram,size=256M,id=ram-node0 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu,size=768M,id=ram-node1 \
+-numa node,nodeid=1,cpus=1,memdev=ram-node1 \
+-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
\
+-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
new file mode 100644
index 000..35aa2cf
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
@@ -0,0 +1,38 @@
+domain type='qemu'
+  nameSomeDummyHugepagesGuest/name
+  uuidef1bdff4-27f3-4e85-a807-5fb4d58463cc/uuid
+  memory unit='KiB'1048576/memory
+  currentMemory unit='KiB'1048576/currentMemory
+  memoryBacking
+hugepages
+  page size='1048576' unit='KiB' nodeset='1'/
+/hugepages
+  /memoryBacking
+  vcpu placement='static'2/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  cpu
+numa
+  cell id='0' cpus='0' memory='262144'/
+  cell id='1' cpus='1' memory='786432'/
+/numa
+  /cpu
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' 

[libvirt] [PATCH v2 1/6] Introduce virFileFindHugeTLBFS

2014-07-23 Thread Michal Privoznik
This should iterate over mount tab and search for hugetlbfs among with
looking for the default value of huge pages.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/libvirt_private.syms |   2 +
 src/util/virfile.c   | 151 ++-
 src/util/virfile.h   |  12 
 3 files changed, 163 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 51504d1..c928564 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1276,9 +1276,11 @@ virFileDirectFdFlag;
 virFileExists;
 virFileFclose;
 virFileFdopen;
+virFileFindHugeTLBFS;
 virFileFindMountPoint;
 virFileFindResource;
 virFileFindResourceFull;
+virFileGetHugepageSize;
 virFileGetMountReverseSubtree;
 virFileGetMountSubtree;
 virFileHasSuffix;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 463064c..f18a0f5 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2837,6 +2837,9 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 # ifndef CIFS_SUPER_MAGIC
 #  define CIFS_SUPER_MAGIC 0xFF534D42
 # endif
+# ifndef HUGETLBFS_MAGIC
+#  define HUGETLBFS_MAGIC 0x958458f6
+# endif
 
 int
 virFileIsSharedFSType(const char *path,
@@ -2909,14 +2912,158 @@ virFileIsSharedFSType(const char *path,
 
 return 0;
 }
-#else
+
+int
+virFileGetHugepageSize(const char *path,
+   unsigned long long *size)
+{
+int ret = -1;
+struct statfs fs;
+
+if (statfs(path, fs)  0) {
+virReportSystemError(errno,
+ _(cannot determine filesystem for '%s'),
+ path);
+goto cleanup;
+}
+
+if (fs.f_type != HUGETLBFS_MAGIC) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(not a hugetlbfs mount: '%s'),
+   path);
+goto cleanup;
+}
+
+*size = fs.f_bsize / 1024; /* we are storing size in KiB */
+ret = 0;
+ cleanup:
+return ret;
+}
+
+# define PROC_MEMINFO /proc/meminfo
+# define HUGEPAGESIZE_STR Hugepagesize:
+
+static int
+virFileGetDefaultHugepageSize(unsigned long long *size)
+{
+int ret = -1;
+char *meminfo, *c, *n, *unit;
+
+if (virFileReadAll(PROC_MEMINFO, 4096, meminfo)  0)
+goto cleanup;
+
+if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Unable to parse %s),
+   PROC_MEMINFO);
+goto cleanup;
+}
+c += strlen(HUGEPAGESIZE_STR);
+
+if ((n = strchr(c, '\n'))) {
+/* Cut off the rest of the meminfo file */
+*n = '\0';
+}
+
+if (virStrToLong_ull(c, unit, 10, size)  0 || STRNEQ(unit,  kB)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Unable to parse %s %s),
+   HUGEPAGESIZE_STR, c);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(meminfo);
+return ret;
+}
+
+# define PROC_MOUNTS /proc/mounts
+
+int
+virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
+ size_t *ret_nfs)
+{
+int ret = -1;
+FILE *f = NULL;
+struct mntent mb;
+char mntbuf[1024];
+virHugeTLBFSPtr fs = NULL;
+size_t nfs = 0;
+unsigned long long default_hugepagesz;
+
+if (virFileGetDefaultHugepageSize(default_hugepagesz)  0)
+goto cleanup;
+
+if (!(f = setmntent(PROC_MOUNTS, r))) {
+virReportSystemError(errno,
+ _(Unable to open %s),
+ PROC_MOUNTS);
+goto cleanup;
+}
+
+while (getmntent_r(f, mb, mntbuf, sizeof(mntbuf))) {
+virHugeTLBFSPtr tmp;
+
+if (STRNEQ(mb.mnt_type, hugetlbfs))
+continue;
+
+if (VIR_REALLOC_N(fs, nfs + 1)  0)
+goto cleanup;
+
+tmp = fs[nfs];
+nfs++;
+
+if (VIR_STRDUP(tmp-mnt_dir, mb.mnt_dir)  0)
+goto cleanup;
+
+if (virFileGetHugepageSize(tmp-mnt_dir, tmp-size)  0)
+goto cleanup;
+
+tmp-deflt = tmp-size == default_hugepagesz;
+}
+
+*ret_fs = fs;
+*ret_nfs = nfs;
+fs = NULL;
+nfs = 0;
+ret = 0;
+
+ cleanup:
+endmntent(f);
+while (nfs)
+VIR_FREE(fs[--nfs].mnt_dir);
+VIR_FREE(fs);
+return ret;
+}
+
+#else /* defined __linux__ */
+
 int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED,
   int fstypes ATTRIBUTE_UNUSED)
 {
 /* XXX implement me :-) */
 return 0;
 }
-#endif
+
+int
+virFileGetHugepageSize(const char *path ATTRIBUTE_UNUSED,
+   unsigned long long *size ATTRIBUTE_UNUSED)
+{
+/* XXX implement me :-) */
+virReportUnsupportedError();
+return -1;
+}
+
+int
+virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs ATTRIBUTE_UNUSED,
+ size_t *ret_nfs ATTRIBUTE_UNUSED)
+{
+/* XXX implement me :-) */
+virReportUnsupportedError();
+return -1;
+}
+#endif /* defined __linux__ */
 
 int 

[libvirt] [PATCH v2 5/6] qemu: Implement ./hugepages/page/[@size, @unit, @nodeset]

2014-07-23 Thread Michal Privoznik
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/qemu/qemu_capabilities.c   |  2 +
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 91 +++---
 .../qemuxml2argv-hugepages-pages.args  | 16 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
 tests/qemuxml2argvtest.c   | 10 ++-
 6 files changed, 109 insertions(+), 13 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 07306e5..f69c4d0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -263,6 +263,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   memory-backend-ram, /* 170 */
   numa,
+  memory-backend-file,
 );
 
 
@@ -1481,6 +1482,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { pvpanic, QEMU_CAPS_DEVICE_PANIC },
 { usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
 { memory-backend-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
+{ memory-backend-file, QEMU_CAPS_OBJECT_MEMORY_FILE },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 4332633..e80a377 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -211,6 +211,7 @@ typedef enum {
 QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in 
metadata */
 QEMU_CAPS_OBJECT_MEMORY_RAM  = 170, /* -object memory-backend-ram */
 QEMU_CAPS_NUMA   = 171, /* newer -numa handling with disjoint 
cpu ranges */
+QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d5f5f02..81be42a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6381,24 +6381,36 @@ qemuBuildSmpArgStr(const virDomainDef *def,
 }
 
 static int
-qemuBuildNumaArgStr(const virDomainDef *def,
+qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
+const virDomainDef *def,
 virCommandPtr cmd,
 virQEMUCapsPtr qemuCaps)
 {
-size_t i;
+size_t i, j;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
+virDomainHugePagePtr master_hugepage = NULL;
 char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
 char *nodemask = NULL;
+char *mem_path = NULL;
 int ret = -1;
 
 if (virDomainNumatuneHasPerNodeBinding(def-numatune) 
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
+  virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Per-node memory binding is not supported 
  with this QEMU));
 goto cleanup;
 }
 
+if (def-mem.nhugepages  def-mem.hugepages[0].size 
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(huge pages pre NUMA node are not 
+ supported with this QEMU));
+goto cleanup;
+}
+
 for (i = 0; i  def-cpu-ncells; i++) {
 int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
 def-cpu-cells[i].mem = cellmem * 1024;
@@ -6417,15 +6429,74 @@ qemuBuildNumaArgStr(const virDomainDef *def,
 goto cleanup;
 }
 
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
 virDomainNumatuneMemMode mode;
+virDomainHugePagePtr hugepage = NULL;
 const char *policy = NULL;
 
 mode = virDomainNumatuneGetMode(def-numatune, i);
 policy = qemuNumaPolicyTypeToString(mode);
 
-virBufferAsprintf(buf, 
memory-backend-ram,size=%dM,id=ram-node%zu,
-  cellmem, i);
+/* Find the huge page size we want to use */
+for (j = 0; j  def-mem.nhugepages; j++) {
+bool thisHugepage = false;
+
+hugepage = def-mem.hugepages[j];
+
+if (!hugepage-nodemask) {
+master_hugepage = hugepage;
+continue;
+}
+
+if (virBitmapGetBit(hugepage-nodemask, i, thisHugepage)  0) 
{
+/* Ignore this error. It's not an error after all. Well,
+ * the nodemask for this page/ can contain lower NUMA
+ * nodes than we are querying in here. */
+continue;
+ 

[libvirt] [PATCH v2 3/6] virbitmap: Introduce virBitmapOverlaps

2014-07-23 Thread Michal Privoznik
This internal API just checks if two bitmaps intersect or not.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---

Notes:
This has been ACKed already, but the name of the function was not quite 
right.
But there's no functional change since then.

 src/libvirt_private.syms |  1 +
 src/util/virbitmap.c | 20 
 src/util/virbitmap.h |  3 +++
 tests/virbitmaptest.c| 26 ++
 4 files changed, 50 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c928564..8b50417 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1005,6 +1005,7 @@ virBitmapNewCopy;
 virBitmapNewData;
 virBitmapNextClearBit;
 virBitmapNextSetBit;
+virBitmapOverlaps;
 virBitmapParse;
 virBitmapSetAll;
 virBitmapSetBit;
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 1029635..27282df 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -732,3 +732,23 @@ virBitmapDataToString(void *data,
 virBitmapFree(map);
 return ret;
 }
+
+bool
+virBitmapOverlaps(virBitmapPtr b1,
+  virBitmapPtr b2)
+{
+size_t i;
+
+if (b1-max_bit  b2-max_bit) {
+virBitmapPtr tmp = b1;
+b1 = b2;
+b2 = tmp;
+}
+
+for (i = 0; i  b1-map_len; i++) {
+if (b1-map[i]  b2-map[i])
+return true;
+}
+
+return false;
+}
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 142a218..4493cc9 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -114,5 +114,8 @@ size_t virBitmapCountBits(virBitmapPtr bitmap)
 char *virBitmapDataToString(void *data,
 int len)
 ATTRIBUTE_NONNULL(1);
+bool virBitmapOverlaps(virBitmapPtr b1,
+   virBitmapPtr b2)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 #endif
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 048946f..ea832ad 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -510,6 +510,30 @@ test9(const void *opaque ATTRIBUTE_UNUSED)
 }
 
 static int
+test10(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
+
+if (virBitmapParse(0-3,5-8,11-15, 0, b1, 20)  0 ||
+virBitmapParse(4,9,10,16-19, 0, b2, 20)  0 ||
+virBitmapParse(15, 0, b3, 20)  0)
+goto cleanup;
+
+if (virBitmapOverlaps(b1, b2) ||
+virBitmapOverlaps(b2, b3) ||
+!virBitmapOverlaps(b1, b3))
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virBitmapFree(b1);
+virBitmapFree(b2);
+virBitmapFree(b3);
+return ret;
+}
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -532,6 +556,8 @@ mymain(void)
 ret = -1;
 if (virtTestRun(test9, test9, NULL)  0)
 ret = -1;
+if (virtTestRun(test10, test10, NULL)  0)
+ret = -1;
 
 return ret;
 }
-- 
1.8.5.5

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


[libvirt] [PATCH v2 4/6] domain: Introduce ./hugepages/page/[@size, @unit, @nodeset]

2014-07-23 Thread Michal Privoznik
  memoryBacking
hugepages
  page size=1 unit=G nodeset=0-3,5/
  page size=2 unit=M nodeset=4/
/hugepages
  /memoryBacking

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 docs/formatdomain.html.in  |  18 +-
 docs/schemas/domaincommon.rng  |  19 +-
 src/conf/domain_conf.c | 197 +++--
 src/conf/domain_conf.h |  13 +-
 src/parallels/parallels_driver.c   |   2 +-
 src/qemu/qemu_command.c|   2 +-
 src/qemu/qemu_process.c|   2 +-
 .../qemuxml2argv-hugepages-pages.xml   |  45 +
 tests/qemuxml2xmltest.c|   1 +
 9 files changed, 277 insertions(+), 22 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8950959..bce5885 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -617,7 +617,9 @@
 lt;domaingt;
   ...
   lt;memoryBackinggt;
-lt;hugepages/gt;
+lt;hugepagesgt;
+  lt;page size=1 unit=G nodeset=0-3,5/gt;
+  lt;page size=2 unit=M nodeset=4/gt;
 lt;nosharepages/gt;
 lt;locked/gt;
   lt;/memoryBackinggt;
@@ -632,7 +634,19 @@
 dl
   dtcodehugepages/code/dt
   ddThis tells the hypervisor that the guest should have its memory
-allocated using hugepages instead of the normal native page size./dd
+  allocated using hugepages instead of the normal native page size.
+  span class='since'Since 1.2.5/span it's possible to set hugepages
+  more specifically per numa node. The codepage/code element is
+  introduced. It has one compulsory attribute codesize/code which
+  specifies which hugepages should be used (especially useful on systems
+  supporting hugepages of different sizes). The default unit for the
+  codesize/code attribute is kilobytes (multiplier of 1024). If you
+  want to use different unit, use optional codeunit/code attribute.
+  For systems with NUMA, the optional codenodeset/code attribute may
+  come handy as it ties given guest's NUMA nodes to certain hugepage
+  sizes. From the example snippet, one gigabyte hugepages are used for
+  every NUMA node except node number four. For the correct syntax see
+  a href=#elementsNUMATuningthis/a./dd
   dtcodenosharepages/code/dt
   ddInstructs hypervisor to disable shared pages (memory merge, KSM) for
 this domain. span class=sinceSince 1.0.6/span/dd
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f6f697c..cf4cda8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -567,7 +567,24 @@
   interleave
 optional
   element name=hugepages
-empty/
+zeroOrMore
+  element name=page
+attribute name=size
+  ref name=unsignedLong/
+/attribute
+optional
+  attribute name='unit'
+ref name='unit'/
+  /attribute
+/optional
+optional
+  attribute name=nodeset
+ref name='cpuset'/
+  /attribute
+/optional
+empty/
+  /element
+/zeroOrMore
   /element
 /optional
 optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8d1fe7..ff50252 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11260,6 +11260,57 @@ virDomainParseMemory(const char *xpath, 
xmlXPathContextPtr ctxt,
 }
 
 
+static int
+virDomainHugepagesParseXML(xmlNodePtr node,
+   xmlXPathContextPtr ctxt,
+   virDomainHugePagePtr hugepage)
+{
+int ret = -1;
+xmlNodePtr oldnode = ctxt-node;
+unsigned long long bytes, max;
+char *unit = NULL, *nodeset = NULL;
+
+ctxt-node = node;
+
+/* On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63).  */
+if (sizeof(unsigned long)  sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
+if (virXPathULongLong(string(./@size), ctxt, bytes)  0) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+   _(unable to parse size attribute));
+goto cleanup;
+}
+
+unit = virXPathString(string(./@unit), ctxt);
+
+if (virScaleInteger(bytes, unit, 1024, max)  0)
+goto cleanup;
+
+if (!(hugepage-size = VIR_DIV_UP(bytes, 1024))) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+   _(hugepage size can't be zero));
+goto cleanup;
+}
+
+if 

[libvirt] [PATCH v2] qemuConnectGetDomainCapabilities: Use wiser defaults

2014-07-23 Thread Michal Privoznik
Up to now, users have to pass two arguments at least: domain virt type
('qemu' vs 'kvm') and one of emulatorbin or architecture. This is not
much user friendly. Nowadays users mostly use KVM and share the host
architecture with the guest. So now, the API (and subsequently virsh
command) can be called with all NULLs  (without any arguments).

Before this patch:
 # virsh domcapabilities
 error: failed to get emulator capabilities
 error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL

 # virsh domcapabilities kvm
 error: failed to get emulator capabilities
 error: invalid argument: at least one of emulatorbin or architecture fields 
must be present

After:

 # virsh domcapabilities
 domainCapabilities
   path/usr/bin/qemu-system-x86_64/path
   domainkvm/domain
   machinepc-i440fx-2.1/machine
   archx86_64/arch
   vcpu max='255'/
 /domainCapabilities

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---

Notes:
It would be nice to have this in the same release as the new API it's 
fixing.
So please, review.

 src/qemu/qemu_driver.c | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b85d909..008f101 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16893,15 +16893,20 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
 virQEMUCapsPtr qemuCaps = NULL;
 int virttype; /* virDomainVirtType */
 virDomainCapsPtr domCaps = NULL;
-int arch = VIR_ARCH_NONE; /* virArch */
+int arch = virArchFromHost(); /* virArch */
 
 virCheckFlags(0, ret);
-virCheckNonNullArgReturn(virttype_str, ret);
 
 if (virConnectGetDomainCapabilitiesEnsureACL(conn)  0)
 return ret;
 
-if ((virttype = virDomainVirtTypeFromString(virttype_str))  0) {
+if (qemuHostdevHostSupportsPassthroughLegacy())
+virttype = VIR_DOMAIN_VIRT_KVM;
+else
+virttype = VIR_DOMAIN_VIRT_QEMU;
+
+if (virttype_str 
+(virttype = virDomainVirtTypeFromString(virttype_str))  0) {
 virReportError(VIR_ERR_INVALID_ARG,
_(unknown virttype: %s),
virttype_str);
@@ -16924,9 +16929,6 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
 
 arch_from_caps = virQEMUCapsGetArch(qemuCaps);
 
-if (arch == VIR_ARCH_NONE)
-arch = arch_from_caps;
-
 if (arch_from_caps != arch) {
 virReportError(VIR_ERR_INVALID_ARG,
_(architecture from emulator '%s' doesn't 
@@ -16935,21 +16937,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
virArchToString(arch));
 goto cleanup;
 }
-} else if (arch_str) {
+} else {
 if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver-qemuCapsCache,
   arch)))
 goto cleanup;
 
-if (!emulatorbin)
-emulatorbin = virQEMUCapsGetBinary(qemuCaps);
-/* Deliberately not checking if provided @emulatorbin matches @arch,
- * since if @emulatorbin was specified the match has been checked a few
- * lines above. */
-} else {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(at least one of emulatorbin or 
- architecture fields must be present));
-goto cleanup;
+emulatorbin = virQEMUCapsGetBinary(qemuCaps);
 }
 
 if (machine) {
-- 
1.8.5.5

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


Re: [libvirt] [PATCH v2] qemuConnectGetDomainCapabilities: Use wiser defaults

2014-07-23 Thread Eric Blake
On 07/23/2014 10:40 AM, Michal Privoznik wrote:
 Up to now, users have to pass two arguments at least: domain virt type
 ('qemu' vs 'kvm') and one of emulatorbin or architecture. This is not
 much user friendly. Nowadays users mostly use KVM and share the host
 architecture with the guest. So now, the API (and subsequently virsh
 command) can be called with all NULLs  (without any arguments).

 
  # virsh domcapabilities
  domainCapabilities
path/usr/bin/qemu-system-x86_64/path
domainkvm/domain
machinepc-i440fx-2.1/machine
archx86_64/arch
vcpu max='255'/
  /domainCapabilities

Nice - it gives a sane answer for the default setup.

 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
 
 Notes:
 It would be nice to have this in the same release as the new API it's 
 fixing.

Agreed.


 +int arch = virArchFromHost(); /* virArch */
  
  virCheckFlags(0, ret);
 -virCheckNonNullArgReturn(virttype_str, ret);
  
  if (virConnectGetDomainCapabilitiesEnsureACL(conn)  0)
  return ret;
  
 -if ((virttype = virDomainVirtTypeFromString(virttype_str))  0) {
 +if (qemuHostdevHostSupportsPassthroughLegacy())
 +virttype = VIR_DOMAIN_VIRT_KVM;
 +else
 +virttype = VIR_DOMAIN_VIRT_QEMU;
 +
 +if (virttype_str 
 +(virttype = virDomainVirtTypeFromString(virttype_str))  0) {
  virReportError(VIR_ERR_INVALID_ARG,
 _(unknown virttype: %s),
 virttype_str);
 @@ -16924,9 +16929,6 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,

Not shown, but arch is reassigned to arch_str if one was provided,
before the next query of arch...

  
  arch_from_caps = virQEMUCapsGetArch(qemuCaps);
  
 -if (arch == VIR_ARCH_NONE)
 -arch = arch_from_caps;
 -
  if (arch_from_caps != arch) {
  virReportError(VIR_ERR_INVALID_ARG,
 _(architecture from emulator '%s' doesn't 

...so this error message is still okay.

ACK.

-- 
Eric Blake   eblake 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 v2 1/6] Introduce virFileFindHugeTLBFS

2014-07-23 Thread Daniel P. Berrange
On Wed, Jul 23, 2014 at 05:37:17PM +0200, Michal Privoznik wrote:
 +int
 +virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
 + size_t *ret_nfs)
 +{
 +int ret = -1;
 +FILE *f = NULL;
 +struct mntent mb;
 +char mntbuf[1024];
 +virHugeTLBFSPtr fs = NULL;
 +size_t nfs = 0;
 +unsigned long long default_hugepagesz;
 +
 +if (virFileGetDefaultHugepageSize(default_hugepagesz)  0)
 +goto cleanup;
 +
 +if (!(f = setmntent(PROC_MOUNTS, r))) {
 +virReportSystemError(errno,
 + _(Unable to open %s),
 + PROC_MOUNTS);
 +goto cleanup;
 +}
 +
 +while (getmntent_r(f, mb, mntbuf, sizeof(mntbuf))) {
 +virHugeTLBFSPtr tmp;
 +
 +if (STRNEQ(mb.mnt_type, hugetlbfs))
 +continue;
 +
 +if (VIR_REALLOC_N(fs, nfs + 1)  0)
 +goto cleanup;
 +
 +tmp = fs[nfs];
 +nfs++;

Stilll think we should be using VIR_EXPAND_N here to ensure
new space is fully zerod.

 +
 +if (VIR_STRDUP(tmp-mnt_dir, mb.mnt_dir)  0)
 +goto cleanup;
 +
 +if (virFileGetHugepageSize(tmp-mnt_dir, tmp-size)  0)
 +goto cleanup;
 +
 +tmp-deflt = tmp-size == default_hugepagesz;
 +}


ACK aside from that.

Regards,
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] [PATCHv1.5 3/8] security: DAC: Remove superfluous link resolution

2014-07-23 Thread Eric Blake
On 07/22/2014 03:20 AM, Peter Krempa wrote:
 When restoring security labels in the dac driver the code would resolve
 the file path and use the resolved one to be chown-ed. The setting code
 doesn't do that. Remove the unnecessary code.

chown() on a symlink changes the underlying file, not the link itself;
you need the BSD extension lchown() to change the owner of a symlink
(and even then, changing the owner of a symlink seldom has any
noticeable impact  - per 'man 7 symlink' on Linux, The only time that
the ownership of a symbolic link matters is  when  the  link is being
removed or renamed in a directory that has the sticky bit set).  So
resolving a symlink before chown()ing it is pointless, since chown()
will resolve it anyways, and we really don't need to care about
lchown().  Likewise, on Linux, chmod() cannot alter a symlink to
anything other than a pointless 0777 access mode.

BSD is a bit different - there, lchown() coupled with chmod() can be
used to alter whether a user can resolve through the symlink in pathname
resolution, depending on the mount parameters of the current file
system.  But this is still a seldom used extension to POSIX.

 ---
  src/security/security_dac.c | 19 +--
  1 file changed, 1 insertion(+), 18 deletions(-)

ACK.

-- 
Eric Blake   eblake 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 v2 1/8] hostdev: Introduce virDomainHostdevSubsysUSB

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Create a separate typedef for the hostdev union data describing USB.
 Then adjust the code to use the new pointer
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |  4 +--
  src/conf/domain_conf.c   | 54 
 +++-
  src/conf/domain_conf.h   | 22 +---
  src/lxc/lxc_cgroup.c |  4 +--
  src/lxc/lxc_controller.c | 10 +++-
  src/lxc/lxc_driver.c | 16 ++--
  src/qemu/qemu_cgroup.c   |  4 +--
  src/qemu/qemu_command.c  | 26 +--
  src/qemu/qemu_hotplug.c  |  7 +++---
  src/security/security_apparmor.c |  6 ++---
  src/security/security_dac.c  | 12 +++--
  src/security/security_selinux.c  | 10 +---
  src/security/virt-aa-helper.c|  5 ++--
  src/util/virhostdev.c| 50 ++---
  14 files changed, 106 insertions(+), 124 deletions(-)
 

ACK

-- 
Eric Blake   eblake 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] numatune: finish the split from domain_conf and remove all dependencies

2014-07-23 Thread Eric Blake
On 07/21/2014 10:21 PM, Martin Kletzander wrote:
 This patch adds back the virDomainDef typedef into domain_conf and
 makes all the numatune_conf functions independent of any virDomainDef
 definitions.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/conf/domain_conf.c   |  9 +-
  src/conf/domain_conf.h   |  2 ++
  src/conf/numatune_conf.c | 78 
 +---
  src/conf/numatune_conf.h | 23 +++---
  src/lxc/lxc_native.c |  8 +++--
  src/qemu/qemu_driver.c   | 10 +--
  6 files changed, 75 insertions(+), 55 deletions(-)
 

ACK.

-- 
Eric Blake   eblake 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 v2 2/8] hostdev: Introduce virDomainHostdevSubsysPCI

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Create a separate typedef for the hostdev union data describing PCI.
 Then adjust the code to use the new pointer
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |  9 
  src/conf/domain_conf.c   | 29 +++--
  src/conf/domain_conf.h   | 12 ++
  src/libxl/libxl_conf.c   |  9 
  src/libxl/libxl_domain.c |  6 +++--
  src/libxl/libxl_driver.c | 34 ++---
  src/qemu/qemu_cgroup.c   | 24 ++--
  src/qemu/qemu_command.c  | 47 
 ++--
  src/qemu/qemu_hotplug.c  | 11 +-
  src/security/security_apparmor.c | 10 -
  src/security/security_dac.c  | 20 +++--
  src/security/security_selinux.c  | 20 +++--
  src/util/virhostdev.c| 34 -
  13 files changed, 128 insertions(+), 137 deletions(-)

ACK.

-- 
Eric Blake   eblake 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] nwfilter: plub memory leak with firewall

2014-07-23 Thread Eric Blake
On 07/22/2014 10:50 PM, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 10:19:54PM -0600, Eric Blake wrote:
 Introduced in commit 70571ccc. Caught by valgrind:

 ==9816== 170 (32 direct, 138 indirect) bytes in 1 blocks are
 definitely lost in loss record 646 of 821
 ==9816==at 0x4A081D4: calloc (in
 /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 ==9816==by 0x50836FB: virAlloc (viralloc.c:144)
 ==9816==by 0x50AEC2B: virFirewallNew (virfirewall.c:204)
 ==9816==by 0x1E2308ED: ebiptablesDriverProbeStateMatch
 (nwfilter_ebiptables_driver.c:3715)
 ==9816==by 0x1E2309AD: ebiptablesDriverInit
 (nwfilter_ebiptables_driver.c:3742)

 * src/nwfilter/nwfilter_ebiptables_driver.c
 (ebiptablesDriverProbeStateMatch): Properly clean up.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 src/nwfilter/nwfilter_ebiptables_driver.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

 
 ACK,

Thanks; pushed.

-- 
Eric Blake   eblake 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] conf: avoid memory leaks while parsing seclabel

2014-07-23 Thread Eric Blake
On 07/22/2014 10:53 PM, Martin Kletzander wrote:
 On Tue, Jul 22, 2014 at 10:08:21PM -0600, Eric Blake wrote:
 Our seclabel parsing was repeatedly assigning malloc'd data into a
 temporary variable, without first freeing the previous use.  Among
 other leaks flagged by valgrind:



 * src/conf/domain_conf.c (virSecurityLabelDefParseXML): Plug leaks
 detected by valgrind.

 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 src/conf/domain_conf.c | 5 +
 1 file changed, 5 insertions(+)

 
 ACK,

Thanks; pushed.

-- 
Eric Blake   eblake 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] maint: simplify some syntax check exemptions

2014-07-23 Thread Eric Blake
On 07/23/2014 12:13 AM, Martin Kletzander wrote:
 On Fri, Jul 18, 2014 at 03:23:51PM -0600, Eric Blake wrote:
 Commit 5028160 accidentally weakened the strtol prohibitions to
 skip ALL files under src/util instead of the former situation of
 just protecting util/virsexpr.c; even though NONE of the files
 in that directory need any protection.

 Shorten some long lines while at it.

 * cfg.mk (exclude_file_name_regexp--sc_prohibit_strtol): No need
 to exclude all of util.
 (exclude_file_name_regexp--sc_prohibit_sprintf): Reduce long line.
 (exclude_file_name_regexp--sc_prohibit_raw_allocation): Likewise.



 +  ^(docs/hacking\.html\.in|.*stp|.pl)$$

 
 This fails for me even though the regexp makes sense.  Either (a)
 adding an asterisk before 'pl' (the same way as for
 '.*stp') or (b) encapsulating the caret in the parentheses should
 work, probably one of those you meant to do.

Oops, serves me right for testing one version, then tweaking, then
posting, without retesting.

 
 ACK with that changed,

Fixed and pushed.

-- 
Eric Blake   eblake 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] spec: drop anything older than Fedora 13

2014-07-23 Thread Eric Blake
Ping.

On 07/15/2014 05:31 PM, Eric Blake wrote:
 RHEL 5 is based on libvirt 0.8.2, as was Fedora 13.  RHEL 5 also
 happens to be the oldest box that we actively support with a
 buildbot, so it is time to clean up some crufty conditionals in
 the spec file that no longer are necessary for modern Fedora.
 
 Although it is probably okay to make further simplifications to
 a newer minimum Fedora version, that can be done as a later patch.
 This patch just focuses on cleaning any comparison of %{?fedora}
 that will always be true or false once we assume a minimum of F13.
 
 * libvirt.spec.in: Make with_audit default to on. Move other
 conditionals to a single RHEL-5 block. Simplify any fedora
 comparison older than 13.  Document our assumptions.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  libvirt.spec.in | 69 
 -
  1 file changed, 24 insertions(+), 45 deletions(-)
 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index 9c7b241..091eec8 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1,5 +1,7 @@
  # -*- rpm-spec -*-
 
 +# This spec file assumes you are building for Fedora 13 or newer,
 +# or for RHEL 5 or newer. It may need some tweaks for other distros.
  # If neither fedora nor rhel was defined, try to guess them from %{dist}
  %if !0%{?rhel}  !0%{?fedora}
  %{expand:%(echo %{?dist} | \
 @@ -122,7 +124,6 @@
  %define with_libpcap   0%{!?_without_libpcap:0}
  %define with_macvtap   0%{!?_without_macvtap:0}
  %define with_libnl 0%{!?_without_libnl:0}
 -%define with_audit 0%{!?_without_audit:0}
  %define with_dtrace0%{!?_without_dtrace:0}
  %define with_cgconfig  0%{!?_without_cgconfig:0}
  %define with_sanlock   0%{!?_without_sanlock:0}
 @@ -136,6 +137,7 @@
 
  # Non-server/HV driver defaults which are always enabled
  %define with_sasl  0%{!?_without_sasl:1}
 +%define with_audit 0%{!?_without_audit:1}
 
 
  # Finally set the OS / architecture specific special cases
 @@ -224,31 +226,21 @@
  %define with_libxl 0
  %endif
 
 -# PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
 -%if 0%{?fedora} = 8 || 0%{?rhel} = 6
 -%define with_polkit0%{!?_without_polkit:1}
 -%endif
 -
 -# libcapng is used to manage capabilities in Fedora 12 / RHEL-6 or newer
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 -%define with_capng 0%{!?_without_capng:1}
 -%endif
 -
  # fuse is used to provide virtualized /proc for LXC
  %if 0%{?fedora} = 17 || 0%{?rhel} = 7
  %define with_fuse  0%{!?_without_fuse:1}
  %endif
 
 -# netcf is used to manage network interfaces in Fedora 12 / RHEL-6 or newer
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 -%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
 -%endif
 -
 -# udev is used to manage host devices in Fedora 12 / RHEL-6 or newer
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 -%define with_udev 0%{!?_without_udev:%{server_drivers}}
 -%else
 +# RHEL 5 lacks newer tools
 +%if 0%{?rhel} == 5
  %define with_hal   0%{!?_without_hal:%{server_drivers}}
 +%else
 +%define with_polkit0%{!?_without_polkit:1}
 +%define with_capng 0%{!?_without_capng:1}
 +%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
 +%define with_udev  0%{!?_without_udev:%{server_drivers}}
 +%define with_yajl  0%{!?_without_yajl:%{server_drivers}}
 +%define with_dtrace 1
  %endif
 
  # interface requires netcf
 @@ -256,11 +248,6 @@
  %define with_interface 0
  %endif
 
 -# Enable yajl library for JSON mode with QEMU
 -%if 0%{?fedora} = 13 || 0%{?rhel} = 6
 -%define with_yajl 0%{!?_without_yajl:%{server_drivers}}
 -%endif
 -
  # Enable sanlock library for lock management with QEMU
  # Sanlock is available only on x86_64 for RHEL
  %if 0%{?fedora} = 16
 @@ -321,16 +308,8 @@
  %define with_libnl 1
  %endif
 
 -%if 0%{?fedora} = 11 || 0%{?rhel} = 5
 -%define with_audit0%{!?_without_audit:1}
 -%endif
 -
 -%if 0%{?fedora} = 13 || 0%{?rhel} = 6
 -%define with_dtrace 1
 -%endif
 -
  # Pull in cgroups config system
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 +%if 0%{?fedora} || 0%{?rhel} = 6
  %if %{with_qemu} || %{with_lxc}
  %define with_cgconfig 0%{!?_without_cgconfig:1}
  %endif
 @@ -350,7 +329,7 @@
 
 
  # Force QEMU to run as non-root
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 +%if 0%{?fedora} || 0%{?rhel} = 6
  %define qemu_user  qemu
  %define qemu_group  qemu
  %else
 @@ -473,7 +452,7 @@ BuildRequires: libattr-devel
  # For pool-build probing for existing pools
  BuildRequires: libblkid-devel = 2.17
  %endif
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 +%if 0%{?fedora} || 0%{?rhel} = 6
  # for augparse, optionally used in testing
  BuildRequires: augeas
  %endif
 @@ -538,7 +517,7 @@ BuildRequires: cyrus-sasl-devel
  %if 0%{?fedora} = 20 || 0%{?rhel} = 7
  BuildRequires: polkit-devel = 0.112
  %else
 -%if 0%{?fedora} = 12 || 0%{?rhel} = 6
 +

Re: [libvirt] [PATCH v2 3/8] hostdev: Introduce virDomainHostdevSubsysSCSI

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Create a separate typedef for the hostdev union data describing SCSI
 Then adjust the code to use the new pointer
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |  7 +++---
  src/conf/domain_conf.c   | 45 ++
  src/conf/domain_conf.h   | 18 --
  src/qemu/qemu_cgroup.c   |  7 +++---
  src/qemu/qemu_command.c  |  7 +++---
  src/qemu/qemu_conf.c | 18 --
  src/qemu/qemu_hotplug.c  | 12 -
  src/security/security_apparmor.c | 10 +++-
  src/security/security_dac.c  | 20 ++-
  src/security/security_selinux.c  | 20 ++-
  src/util/virhostdev.c| 53 
 
  11 files changed, 101 insertions(+), 116 deletions(-)
 

ACK

-- 
Eric Blake   eblake 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 v2] leaseshelper: improvements to support all events

2014-07-23 Thread Eric Blake
On 07/22/2014 09:20 AM, Peter Krempa wrote:
 On 07/22/14 01:03, Nehal J Wani wrote:
 This patch enables the helper program to detect event(s) triggered when there
 is a change in lease length or expiry and client-id. This transfers complete
 control of leases database to libvirt and obsoletes use of the lease database
 file (network-name.leases). That file will not be created, read, or 
 written.
 This is achieved by adding the option --leasefile-ro to dnsmasq and passing a
 custom env var to leaseshelper, which helps us map events related to leases
 with their corresponding network bridges, no matter what the event be.

 Also, this requires the addition of a new non-lease entry in our custom lease
 database: server-duid. It is required to identify a DHCPv6 server.

 Now that dnsmasq doesn't maintain its own leases database, it relies on our
 helper program to tell it about previous leases and server duid. Thus, this
 patch makes our leases program honor an extra action: init, in which it 
 sends
 the known info in a particular format to dnsmasq by printing it to stdout.

 ---
  This is compatible with libvirt 1.2.6 as only additions have been
  introduced, and the old leases file (*.staus) will still be supported.

s/staus/status/


 +else {
 +if (add) {
 +if (virJSONValueArrayAppend(leases_array_new, lease_new)  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(failed to create json));
 +goto cleanup;
 +}
 +lease_new = NULL;
 +}
 +
 +if (server_duid) {
 +if (!(lease_new = virJSONValueNewObject())) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(failed to create json));
 +goto cleanup;
 +}
 +
 +if (virJSONValueObjectAppendString(lease_new,
 +   server-duid, server_duid) 
  0)
 +goto cleanup;
 
 Hmm this is really odd. Why is the server_duid stored as a part of the
 leases array as it's a completely separate info and occuring only once?

Indeed.  The pre-patch file looks like:

[
{
iaid: 1221229,
ip-address: 2001:db8:ca2:2:1::95,
mac-address: 52:54:00:12:a2:6d,
hostname: Fedora20,
client-id:
00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55
expiry-time: 1393244216
},
...
]

I think the enhanced format should look like:

{
  server-duid: ...,
  leases: [
{
iaid: 1221229,
ip-address: 2001:db8:ca2:2:1::95,
mac-address: 52:54:00:12:a2:6d,
hostname: Fedora20,
client-id:
00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55
expiry-time: 1393244216
},
...
]
}

which means that when you first parse the file, the new code has to
determine if the parsed JSON item is an object (new style) or an array
(old style); and grab out the array from either style for the rest of
the code to manipulate.

 
 Unfortunately, looking at the format of the lease file the array of
 leases is the top level element. Is there a way we could (without
 complicating the code too much) convert it to a object so that it's
 extensible?

I agree - it's better to rearrange the file to place the new content as
a sibling to the existing content by creating another wrapper layer
around both, rather than commandeering an array slot for non-lease
information.

 
 The change to using the ENV variable has turned out great, unfortunately
 there's a problem with the lease file JSON we need to clear before
 proceeding.

Looking forward to v3.

-- 
Eric Blake   eblake 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 v2] leaseshelper: improvements to support all events

2014-07-23 Thread Eric Blake
On 07/23/2014 02:32 PM, Eric Blake wrote:
 
 I think the enhanced format should look like:
 
 {
   server-duid: ...,
   leases: [
   {
 iaid: 1221229,
 ip-address: 2001:db8:ca2:2:1::95,
 mac-address: 52:54:00:12:a2:6d,
 hostname: Fedora20,
 client-id:
 00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55
 expiry-time: 1393244216
 },
 ...
 ]
 }

Another idea: make it easier for any future changes.  In your top-level
object, add some other fields:

{
  file-format: 1,
  comment: leases file generated by leaseshelper from libvirt 1.2.7,
  server-duid...

where file-format was implicitly 0 for the old style, and where you can
bump it to 2 if we have to make future changes; and where the comment is
free-form (human-readable, not machine-parseable) information that makes
it easier to debug what the file contains and which build of
leaseshelper generated it (since the leaseshelper version might be
different than libvirtd).


-- 
Eric Blake   eblake 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 v2] leaseshelper: improvements to support all events

2014-07-23 Thread Eric Blake
On 07/23/2014 02:32 PM, Eric Blake wrote:
 I think the enhanced format should look like:
 
 {
   server-duid: ...,
   leases: [
   {
 iaid: 1221229,
 ip-address: 2001:db8:ca2:2:1::95,
 mac-address: 52:54:00:12:a2:6d,
 hostname: Fedora20,
 client-id:
 00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55
 expiry-time: 1393244216
 },
 ...
 ]

On IRC, we discussed another alternative - keep the top-level item as an
array, and instead of adding server-duid as an array element, just add
it as an optional field member of each {} ipv6 lease in the array
(multiple copies of the string, but oh well).  At least that way, you
aren't artificially adding a non-lease to the array itself, and
hopefully libvirt 1.2.6 ignores unknown fields of a lease array entry.

-- 
Eric Blake   eblake 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] libvirt log format

2014-07-23 Thread Yuanzhen Gu
Hello folks,

Does any one know how to read the libvirt log information?

For example,

2014-07-22 17:25:22.984+: 18571: debug : qemuMonitorIOProcess:356 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f733c000a40 buf={timestamp: {seconds:
1406049922, microseconds: 983916}, event: STOP}

is there anyway to find the qemuMonitorIOProcess() function in the source
code?  and what does 356 after that means?   Thanks!

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

Re: [libvirt] libvirt log format

2014-07-23 Thread Eric Blake
On 07/23/2014 03:23 PM, Yuanzhen Gu wrote:
 Hello folks,
 
 Does any one know how to read the libvirt log information?
 
 For example,
 
 2014-07-22 17:25:22.984+: 18571: debug : qemuMonitorIOProcess:356 :
 QEMU_MONITOR_IO_PROCESS: mon=0x7f733c000a40 buf={timestamp: {seconds:
 1406049922, microseconds: 983916}, event: STOP}
 
 is there anyway to find the qemuMonitorIOProcess() function in the source
 code?  and what does 356 after that means?   Thanks!

I just answered your same question on the libvirt-users list; but in
short, 'git grep' is your friend, and 356 is the line number of the code
emitting the log message at the state of the source code used to build
the binary you are running.

-- 
Eric Blake   eblake 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 v2] leaseshelper: improvements to support all events

2014-07-23 Thread Nehal J Wani
 On IRC, we discussed another alternative - keep the top-level item as an
 array, and instead of adding server-duid as an array element, just add
 it as an optional field member of each {} ipv6 lease in the array
 (multiple copies of the string, but oh well).  At least that way, you
 aren't artificially adding a non-lease to the array itself, and
 hopefully libvirt 1.2.6 ignores unknown fields of a lease array entry.

This seems to be the most easy option. Also, how about adding another
field to every lease:
generated-by: leaseshelper v$X (where X is the value provided by
the macro PACKAGE_VERSION)
so that whenever a new field is added in the future, we don't have
trouble finding out which version of leaseshelper added it.


Regards,
Nehal J Wani

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


[libvirt] [PATCH] leaseshelper: avoid mem leak after storing lease entries

2014-07-23 Thread Nehal J Wani
Contents of existing lease file were being stored in a variable
which was never freed.

---
 src/network/leaseshelper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index e4b5283..c8543a2 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -361,6 +361,7 @@ main(int argc, char **argv)
 
 VIR_FREE(pid_file);
 VIR_FREE(exptime);
+VIR_FREE(lease_entries);
 VIR_FREE(custom_lease_file);
 virJSONValueFree(lease_new);
 virJSONValueFree(leases_array);
-- 
1.9.3

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


Re: [libvirt] [PATCH v2 4/8] hostdev: Introduce virDomainHostdevSubsysSCSIHost

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Split virDomainHostdevSubsysSCSI further. In preparation for having
 either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI
 to contain just a virDomainHostdevSubsysSCSIHost to describe the
 'scsi_host' host device
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |  8 --
  src/conf/domain_conf.c   | 59 
 
  src/conf/domain_conf.h   | 14 --
  src/qemu/qemu_cgroup.c   |  9 --
  src/qemu/qemu_command.c  |  7 +++--
  src/qemu/qemu_conf.c | 18 ++--
  src/qemu/qemu_hotplug.c  | 13 +
  src/security/security_apparmor.c |  5 ++--
  src/security/security_dac.c  | 10 ---
  src/security/security_selinux.c  | 10 ---
  src/util/virhostdev.c| 36 
  11 files changed, 113 insertions(+), 76 deletions(-)

ACK

-- 
Eric Blake   eblake 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/2] add support for --config in setmaxmem command

2014-07-23 Thread chenhanx...@cn.fujitsu.com
ping

 -Original Message-
 From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
 On Behalf Of Chen Hanxiao
 Sent: Wednesday, July 16, 2014 5:51 PM
 To: libvir-list@redhat.com
 Subject: [libvirt] [PATCH 0/2] add support for --config in setmaxmem command
 
 
 Chen Hanxiao (2):
   LXC: add support for --config in setmaxmem command
   LXC: use lxcDomainSetMemoryFlags to do lxcDomainSetMaxMemory's work
 
  src/lxc/lxc_driver.c | 105
 +--
  1 file changed, 51 insertions(+), 54 deletions(-)
 
 --
 1.9.0
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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


Re: [libvirt] [PATCH] leaseshelper: avoid mem leak after storing lease entries

2014-07-23 Thread Eric Blake
On 07/23/2014 04:05 PM, Nehal J Wani wrote:
 Contents of existing lease file were being stored in a variable
 which was never freed.
 
 ---
  src/network/leaseshelper.c | 1 +
  1 file changed, 1 insertion(+)

ACK and pushed.

 
 diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
 index e4b5283..c8543a2 100644
 --- a/src/network/leaseshelper.c
 +++ b/src/network/leaseshelper.c
 @@ -361,6 +361,7 @@ main(int argc, char **argv)
  
  VIR_FREE(pid_file);
  VIR_FREE(exptime);
 +VIR_FREE(lease_entries);
  VIR_FREE(custom_lease_file);
  virJSONValueFree(lease_new);
  virJSONValueFree(leases_array);
 

-- 
Eric Blake   eblake 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 v2 5/8] Add virConnectPtr for qemuBuildSCSIHostdevDrvStr

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Add a conn for future patches to be able to grab the secret when
 authenticating an iSCSI host device
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_command.c |  5 +++--
  src/qemu/qemu_command.h |  5 +++--
  src/qemu/qemu_driver.c  |  6 +++---
  src/qemu/qemu_hotplug.c | 34 --
  src/qemu/qemu_hotplug.h |  9 ++---
  5 files changed, 35 insertions(+), 24 deletions(-)
 

 +++ b/src/qemu/qemu_command.h
 @@ -170,10 +170,11 @@ char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
   virDomainHostdevDefPtr dev,
   virQEMUCapsPtr qemuCaps);
  
 -char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
 +char * qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
 +  virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps,
qemuBuildCommandLineCallbacksPtr callbacks)
 -ATTRIBUTE_NONNULL(3);
 +ATTRIBUTE_NONNULL(4);
  char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,

While touching these two declarations, you could get rid of the space
after * to be consistent to the style.

ACK

-- 
Eric Blake   eblake 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] [PATCHv4 0/2] lxc keep/drop capabilities

2014-07-23 Thread Gao feng
On 07/23/2014 04:18 PM, Peter Krempa wrote:
 On 07/23/14 09:17, Gao feng wrote:
 On 07/18/2014 04:02 PM, Cédric Bosdonnat wrote:
 Hi all,

 Even though the v3 has been ACKed (but not pushed), I rebased it on top of
 master before pushing it and did a few changes that are worth checking 
 before.

  * Path 1 (feature) and 3 (doc) of the previous version merge merged 
 together as
suggested for another similar series.

  * virCgroupAllowDevice() has been changed to use negative major / minor 
 device
values to output '*'. I never saw any of them negative, but I don't have 
 a
good knowledge of that.



 ACK  Pushed, thanks!
 
 Please compile the code before pushing! This broke the build:
 

oops! My fault, will take care next time.

 
  CC   lxc/libvirt_driver_lxc_impl_la-lxc_cgroup.lo
 In file included from /usr/include/cap-ng.h:27:0,
  from lxc/lxc_container.c:48:
 lxc/lxc_container.c: In function 'lxcContainerDropCapabilities':
 lxc/lxc_container.c:1951:35: error: comparison of unsigned expression = 0 is 
 always true [-Werror=type-limits]
  if (!cap_valid(capsMapping[i]))
^
 cc1: all warnings being treated as errors
 make[3]: *** [lxc/libvirt_driver_lxc_impl_la-lxc_container.lo] Error 1
 make[3]: *** Waiting for unfinished jobs
 lxc/lxc_cgroup.c: In function 'virLXCCgroupSetupDeviceACL':
 lxc/lxc_cgroup.c:354:9: error: jump skips variable initialization 
 [-Werror=jump-misses-init]
  goto cleanup;
  ^
 lxc/lxc_cgroup.c:456:2: note: label 'cleanup' defined here
   cleanup:
   ^
 lxc/lxc_cgroup.c:357:9: note: 'capMknod' declared here
  int capMknod = def-caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD];
  ^
 cc1: all warnings being treated as errors
 
 
 Fixup patch comming soon.


Thanks!


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

Re: [libvirt] [PATCH v2 6/8] hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI

2014-07-23 Thread Eric Blake
On 07/21/2014 02:47 PM, John Ferlan wrote:
 Create the structures and API's to hold and manage the iSCSI host device.
 This extends the 'scsi_host' definitions added in commit id '5c811dce'.
 A future patch will add the XML parsing, but that code requires some
 infrastructure to be in place first in order to handle the differences
 between a 'scsi_host' and an 'iSCSI host' device.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |  20 +++-
  src/conf/domain_conf.c   |  47 -
  src/conf/domain_conf.h   |  20 
  src/qemu/qemu_cgroup.c   |  35 ---
  src/qemu/qemu_command.c  |  74 ---
  src/qemu/qemu_hotplug.c  |  36 +--
  src/security/security_apparmor.c |   6 ++
  src/security/security_dac.c  |  12 +++
  src/security/security_selinux.c  |  12 +++
  src/util/virhostdev.c| 200 
 +--
  10 files changed, 349 insertions(+), 113 deletions(-)
 

  static int
 +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first,
 + virDomainHostdevDefPtr second)
 +{
 +virDomainHostdevSubsysSCSIiSCSIPtr first_iscsisrc =
 +first-source.subsys.u.scsi.u.iscsi;
 +virDomainHostdevSubsysSCSIiSCSIPtr second_iscsisrc =
 +second-source.subsys.u.scsi.u.iscsi;
 +
 +if (STREQ(first_iscsisrc-hosts[0].name, second_iscsisrc-hosts[0].name) 
 

Do you need to match nhosts, or is nhosts always 1?


 +
 +/* Only delete the devices which are marked as being used by @name,
 + * because qemuProcessStart could fail on the half way. */

Sounds funny; maybe could fail half way through


 -
 -/* Only delete the devices which are marked as being used by @name,
 - * because qemuProcessStart could fail on the half way. */

Then again, it's just code motion.

Conditional ACK, if the match function doesn't need to track nhosts.


-- 
Eric Blake   eblake 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] util: virTimeFieldsThenRaw never return negative

2014-07-23 Thread Wang Rui
From: James james.wangyu...@huawei.com

virTimeFieldsThenRaw will never return negative result, so I delete related
judgement.

Signed-off-by: James james.wangyu...@huawei.com
---
 src/util/virtime.c | 7 ++-
 src/util/virtime.h | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/util/virtime.c b/src/util/virtime.c
index 2a91ea5..662c161 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -121,9 +121,8 @@ const unsigned short int __mon_yday[2][13] = {
  * Converts the timestamp @when into broken-down field format.
  * Time time is always in UTC
  *
- * Returns 0 on success, -1 on error with errno set
  */
-int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
+void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
 {
 /* This code is taken from GLibC under terms of LGPLv2+ */
 long int days, rem, y;
@@ -171,7 +170,6 @@ int virTimeFieldsThenRaw(unsigned long long when, struct tm 
*fields)
 days -= ip[y];
 fields-tm_mon = y;
 fields-tm_mday = days + 1;
-return 0;
 }
 
 
@@ -209,8 +207,7 @@ int virTimeStringThenRaw(unsigned long long when, char *buf)
 {
 struct tm fields;
 
-if (virTimeFieldsThenRaw(when, fields)  0)
-return -1;
+virTimeFieldsThenRaw(when, fields);
 
 fields.tm_year += 1900;
 fields.tm_mon += 1;
diff --git a/src/util/virtime.h b/src/util/virtime.h
index 25332db..61f36dc 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -43,12 +43,12 @@ int virTimeMillisNowRaw(unsigned long long *now)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 int virTimeFieldsNowRaw(struct tm *fields)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
-ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 int virTimeStringNowRaw(char *buf)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 int virTimeStringThenRaw(unsigned long long when, char *buf)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
 /* These APIs are *not* async signal safe and return -1,
  * raising a libvirt error on failure
-- 
1.7.12.4


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


[libvirt] [PATCH v2 3/3] nodedev: fix pci express memory leak

2014-07-23 Thread Eric Blake
Leak introduced in commit 16ebf10f (v1.2.6), detected by valgrind:

==9816== 216 (96 direct, 120 indirect) bytes in 6 blocks are definitely lost in 
loss record 665 of 821
==9816==at 0x4A081D4: calloc (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9816==by 0x50836FB: virAlloc (viralloc.c:144)
==9816==by 0x1DBDBE27: udevProcessPCI (node_device_udev.c:546)
==9816==by 0x1DBDD79D: udevGetDeviceDetails (node_device_udev.c:1293)

* src/util/virpci.h (virPCIEDeviceInfoFree): New prototype.
* src/util/virpci.c (virPCIEDeviceInfoFree): New function.
* src/conf/node_device_conf.c (virNodeDevCapsDefFree): Clear
pci_express under pci case.
(virNodeDevCapPCIDevParseXML): Avoid leak.
* src/node_device/node_device_udev.c (udevProcessPCI): Likewise.
* src/libvirt_private.syms (virpci.h): Export it.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/conf/node_device_conf.c|  3 ++-
 src/libvirt_private.syms   |  1 +
 src/node_device/node_device_udev.c |  2 +-
 src/util/virpci.c  | 12 
 src/util/virpci.h  |  3 +++
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index b244a1f..78bc63f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1291,7 +1291,7 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt,

 ret = 0;
  out:
-VIR_FREE(pci_express);
+virPCIEDeviceInfoFree(pci_express);
 ctxt-node = orignode;
 return ret;
 }
@@ -1664,6 +1664,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 VIR_FREE(data-pci_dev.iommuGroupDevices[i]);
 }
 VIR_FREE(data-pci_dev.iommuGroupDevices);
+virPCIEDeviceInfoFree(data-pci_dev.pci_express);
 break;
 case VIR_NODE_DEV_CAP_USB_DEV:
 VIR_FREE(data-usb_dev.product_name);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 51504d1..fe02f9c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1753,6 +1753,7 @@ virPCIDeviceSetUnbindFromStub;
 virPCIDeviceSetUsedBy;
 virPCIDeviceUnbind;
 virPCIDeviceWaitForCleanup;
+virPCIEDeviceInfoFree;
 virPCIGetNetName;
 virPCIGetPhysicalFunction;
 virPCIGetVirtualFunctionIndex;
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 28d2953..0fe474d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -570,7 +570,7 @@ static int udevProcessPCI(struct udev_device *device,

  out:
 virPCIDeviceFree(pciDev);
-VIR_FREE(pci_express);
+virPCIEDeviceInfoFree(pci_express);
 return ret;
 }

diff --git a/src/util/virpci.c b/src/util/virpci.c
index b7400e9..0098d6c 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2847,3 +2847,15 @@ virPCIDeviceGetLinkCapSta(virPCIDevicePtr dev,
 virPCIDeviceConfigClose(dev, fd);
 return ret;
 }
+
+
+void
+virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev)
+{
+if (!dev)
+return;
+
+VIR_FREE(dev-link_cap);
+VIR_FREE(dev-link_sta);
+VIR_FREE(dev);
+}
diff --git a/src/util/virpci.h b/src/util/virpci.h
index edec439..3da8eb3 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -212,4 +212,7 @@ int virPCIDeviceGetLinkCapSta(virPCIDevicePtr dev,
   unsigned int *cap_width,
   unsigned int *sta_speed,
   unsigned int *sta_width);
+
+void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev);
+
 #endif /* __VIR_PCI_H__ */
-- 
1.9.3

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


[libvirt] [PATCH v2 1/3] nodedev: let compiler help us on switches

2014-07-23 Thread Eric Blake
The compiler can alert us to places where we need to expand switch
statements because we add a new enum value, but only if we don't
have a default case.

* src/conf/node_device_conf.c (virNodeDeviceDefFormat)
(virNodeDevCapsDefParseXML, virNodeDevCapsDefFree): Drop default
case.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/conf/node_device_conf.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 910a371..ac966d3 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -547,7 +547,6 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_LAST:
-default:
 break;
 }

@@ -1405,7 +1404,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
 case VIR_NODE_DEV_CAP_STORAGE:
 ret = virNodeDevCapStorageParseXML(ctxt, def, node, caps-data);
 break;
-default:
+case VIR_NODE_DEV_CAP_FC_HOST:
+case VIR_NODE_DEV_CAP_VPORTS:
+case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+case VIR_NODE_DEV_CAP_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unknown capability type '%d' for '%s'),
caps-type, def-name);
@@ -1703,7 +1705,6 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_LAST:
-default:
 /* This case is here to shutup the compiler */
 break;
 }
-- 
1.9.3

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


[libvirt] [PATCH v2 0/3] fix pciexpress memory leak

2014-07-23 Thread Eric Blake
diff in v2: split out trivial fixes, refactor pcie code to
virpci.h, add proper free function to cover all leaks

Eric Blake (3):
  nodedev: let compiler help us on switches
  nodedev: move pci express types to virpci.h
  nodedev: fix pci express memory leak

 src/conf/node_device_conf.c| 13 ++---
 src/conf/node_device_conf.h| 29 +
 src/libvirt_private.syms   |  1 +
 src/node_device/node_device_udev.c |  2 +-
 src/util/virpci.c  | 15 +++
 src/util/virpci.h  | 33 -
 6 files changed, 56 insertions(+), 37 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCH v2 2/3] nodedev: move pci express types to virpci.h

2014-07-23 Thread Eric Blake
Finding virPCIE* code is more intuitive if located in virpci.h
instead of node_device_conf.h.

* src/conf/node_device_conf.h (virPCIELinkSpeed, virPCIELink)
(virPCIEDeviceInfo): Move...
* src/util/virpci.h: ...here.
* src/conf/node_device_conf.c (virPCIELinkSpeed): Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/conf/node_device_conf.c |  3 ---
 src/conf/node_device_conf.h | 29 +
 src/util/virpci.c   |  3 +++
 src/util/virpci.h   | 30 +-
 4 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index ac966d3..b244a1f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -58,9 +58,6 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST,
   80203,
   80211)

-VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
-  , 2.5, 5, 8)
-
 static int
 virNodeDevCapsDefParseString(const char *xpath,
  xmlXPathContextPtr ctxt,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index b5bfb7b..fd5d179 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -1,7 +1,7 @@
 /*
  * node_device_conf.h: config handling for node devices
  *
- * Copyright (C) 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2009-2014 Red Hat, Inc.
  * Copyright (C) 2008 Virtual Iron Software, Inc.
  * Copyright (C) 2008 David F. Lively
  *
@@ -81,33 +81,6 @@ typedef enum {
 VIR_NODE_DEV_CAP_FLAG_PCIE  = (1  2),
 } virNodeDevPCICapFlags;

-typedef enum {
-VIR_PCIE_LINK_SPEED_NA = 0,
-VIR_PCIE_LINK_SPEED_25,
-VIR_PCIE_LINK_SPEED_5,
-VIR_PCIE_LINK_SPEED_8,
-VIR_PCIE_LINK_SPEED_LAST
-} virPCIELinkSpeed;
-
-VIR_ENUM_DECL(virPCIELinkSpeed)
-
-typedef struct _virPCIELink virPCIELink;
-typedef virPCIELink *virPCIELinkPtr;
-struct _virPCIELink {
-int port;
-virPCIELinkSpeed speed;
-unsigned int width;
-};
-
-typedef struct _virPCIEDeviceInfo virPCIEDeviceInfo;
-typedef virPCIEDeviceInfo *virPCIEDeviceInfoPtr;
-struct _virPCIEDeviceInfo {
-/* Not all PCI Express devices has link. For example this 'Root Complex
- * Integrated Endpoint' and 'Root Complex Event Collector' don't have it. 
*/
-virPCIELink *link_cap;   /* PCIe device link capabilities */
-virPCIELink *link_sta;   /* Actually negotiated capabilities */
-};
-
 typedef struct _virNodeDevCapsDef virNodeDevCapsDef;
 typedef virNodeDevCapsDef *virNodeDevCapsDefPtr;
 struct _virNodeDevCapsDef {
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 6f4f6af..b7400e9 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -52,6 +52,9 @@ VIR_LOG_INIT(util.pci);
 #define PCI_ID_LEN 10   /*   */
 #define PCI_ADDR_LEN 13 /* :XX:XX.X */

+VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
+  , 2.5, 5, 8)
+
 struct _virPCIDevice {
 unsigned int  domain;
 unsigned int  bus;
diff --git a/src/util/virpci.h b/src/util/virpci.h
index d64c3e2..edec439 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -1,7 +1,7 @@
 /*
  * virpci.h: helper APIs for managing host PCI devices
  *
- * Copyright (C) 2009, 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2009, 2011-2014 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
@@ -26,6 +26,7 @@

 # include internal.h
 # include virobject.h
+# include virutil.h

 typedef struct _virPCIDevice virPCIDevice;
 typedef virPCIDevice *virPCIDevicePtr;
@@ -41,6 +42,33 @@ struct _virPCIDeviceAddress {
 unsigned int function;
 };

+typedef enum {
+VIR_PCIE_LINK_SPEED_NA = 0,
+VIR_PCIE_LINK_SPEED_25,
+VIR_PCIE_LINK_SPEED_5,
+VIR_PCIE_LINK_SPEED_8,
+VIR_PCIE_LINK_SPEED_LAST
+} virPCIELinkSpeed;
+
+VIR_ENUM_DECL(virPCIELinkSpeed)
+
+typedef struct _virPCIELink virPCIELink;
+typedef virPCIELink *virPCIELinkPtr;
+struct _virPCIELink {
+int port;
+virPCIELinkSpeed speed;
+unsigned int width;
+};
+
+typedef struct _virPCIEDeviceInfo virPCIEDeviceInfo;
+typedef virPCIEDeviceInfo *virPCIEDeviceInfoPtr;
+struct _virPCIEDeviceInfo {
+/* Not all PCI Express devices has link. For example this 'Root Complex
+ * Integrated Endpoint' and 'Root Complex Event Collector' don't have it. 
*/
+virPCIELink *link_cap;   /* PCIe device link capabilities */
+virPCIELink *link_sta;   /* Actually negotiated capabilities */
+};
+
 virPCIDevicePtr virPCIDeviceNew(unsigned int domain,
 unsigned int bus,
 unsigned int slot,
-- 
1.9.3

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


[libvirt] [PATCH 2/2] qemu: add support vhost-scsi-pci for device_add hotplug

2014-07-23 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@linux-iscsi.org

This patch adds support for device_add hotplug for vhost-scsi-pci
against QEMU = v1.5.x code.

This includes:

  - Add qemuOpenVhostSCSI() to open fds to /dev/vhost-scsi character
device.
  - Changes to qemuBuildControllerDevStr() for adding 'vhostfd='
parameters to device_add.
  - Add qemuMonitorAddControllerVhost() that is specific to
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VHOST_SCSI
  - Update qemuDomainAttachControllerDevice() to setup vhostfd
fd + name arrays, and invoke qemuOpenVhostSCSI().
  - Update qemuDomainAttachControllerDevice() to invoke
qemuMonitorAddControllerVhost() - qemuMonitorSendFileHandle() -
qemuMonitorAddDevice() in order to pass the pre-opened vhostfd.

This code has been tested using openstack nova volume-attach, using
a Juno v2 development head from 07192014.

Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
Signed-off-by: Mike Perez thin...@gmail.com
---
 src/qemu/qemu_command.c |   56 ---
 src/qemu/qemu_command.h |8 ++-
 src/qemu/qemu_hotplug.c |   47 +--
 src/qemu/qemu_monitor.c |   24 
 src/qemu/qemu_monitor.h |4 
 5 files changed, 133 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 43c0e1c..1e987f9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -678,6 +678,38 @@ static int 
qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
 return 0;
 }
 
+int
+qemuOpenVhostSCSI(virDomainControllerDefPtr controller,
+  int *vhostfd,
+  int *vhostfdSize)
+{
+size_t i;
+
+if (controller-model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VHOST_SCSI) {
+*vhostfdSize = 0;
+return 0;
+}
+
+for (i = 0; i  *vhostfdSize; i++) {
+vhostfd[i] = open(/dev/vhost-scsi, O_RDWR);
+
+if (vhostfd[i]  0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(vhost-scsi was requested for an interface, 

+   but is unavailable));
+goto error;
+}
+}
+
+return 0;
+
+ error:
+while (i--)
+VIR_FORCE_CLOSE(vhostfd[i]);
+
+return -1;
+}
+
 static int
 qemuSetSCSIControllerModel(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
@@ -4153,10 +4185,13 @@ char *
 qemuBuildControllerDevStr(virDomainDefPtr domainDef,
   virDomainControllerDefPtr def,
   virQEMUCapsPtr qemuCaps,
-  int *nusbcontroller)
+  int *nusbcontroller,
+  char **vhostfd,
+  int vhostfdSize)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 int model;
+size_t i;
 
 if (!(def-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI 
   (def-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI ||
@@ -4328,6 +4363,19 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
 if (def-wwpn)
 virBufferAsprintf(buf, ,wwpn=%s, def-wwpn);
 
+if (vhostfdSize) {
+if (vhostfdSize == 1) {
+virBufferAsprintf(buf, ,vhostfd=%s, vhostfd[0]);
+} else {
+virBufferAddLit(buf, ,vhostfds=);
+for (i = 0; i  vhostfdSize; i++) {
+if (i)
+virBufferAddChar(buf, ':');
+virBufferAdd(buf, vhostfd[i], -1);
+}
+}
+}
+
 if (qemuBuildDeviceAddressStr(buf, domainDef, def-info, qemuCaps)  0)
 goto error;
 
@@ -7843,7 +7891,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 virCommandAddArg(cmd, -device);
 if (!(devstr = qemuBuildControllerDevStr(def, cont,
- qemuCaps, 
NULL)))
+ qemuCaps, 
NULL,
+ NULL, 0)))
 goto error;
 
 virCommandAddArg(cmd, devstr);
@@ -7867,7 +7916,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 char *devstr;
 if (!(devstr = qemuBuildControllerDevStr(def, cont, 
qemuCaps,
- usbcontroller)))
+ usbcontroller, 
NULL,
+ 0)))
 goto error;
 
 virCommandAddArg(cmd, devstr);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index cf51056..b0651ff 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -114,6 +114,10 @@ char * qemuBuildNicDevStr(virDomainDefPtr def,
 char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
   

[libvirt] [PATCH 0/2] qemu: add vhost-scsi-pci support

2014-07-23 Thread Nicholas A. Bellinger
From: Nicholas Bellinger n...@linux-iscsi.org

Hi Daniel  Co,

The following is the patch series to add support for vhost-scsi-pci
from QEMU = v1.5.x into libvirt.  It includes proper support for
passing a pre-opened vhostfd into vhost-scsi, which is required in
order to properly support nova performing a device_add for adding
a individual vhost-scsi-pci WWPN endpoint - SCSI controller.

The logic closely follows what vhost-net already does in order to
allow a child qemu process to interact with the vhost-scsi character
device for registering vhost memory, setting vhost-scsi endpoints,
etc.

This code has been tested using openstack nova volume-attach, using
a Juno v2 development head from 07192014.

Thank you,

--nab

Mike Perez (1):
  qemu: add vhost-scsi-pci definitions

Nicholas Bellinger (1):
  qemu: add support vhost-scsi-pci for device_add hotplug

 Makefile.am|2 +-
 docs/formatdomain.html.in  |   20 ++--
 docs/schemas/domaincommon.rng  |5 +
 src/conf/domain_conf.c |   20 +++-
 src/conf/domain_conf.h |2 +
 src/qemu/qemu_capabilities.c   |2 +
 src/qemu/qemu_capabilities.h   |1 +
 src/qemu/qemu_command.c|  115 ++--
 src/qemu/qemu_command.h|8 +-
 src/qemu/qemu_hotplug.c|   47 +++-
 src/qemu/qemu_monitor.c|   24 
 src/qemu/qemu_monitor.h|4 +
 src/vmx/vmx.c  |1 +
 tests/qemucapabilitiesdata/caps_1.5.3-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |1 +
 .../qemuxml2argv-disk-vhost-scsi-cmd_per_lun.args  |9 ++
 .../qemuxml2argv-disk-vhost-scsi-cmd_per_lun.xml   |   29 +
 .../qemuxml2argv-disk-vhost-scsi-max_sectors.args  |9 ++
 .../qemuxml2argv-disk-vhost-scsi-max_sectors.xml   |   29 +
 .../qemuxml2argv-disk-vhost-scsi-num_queues.args   |9 ++
 .../qemuxml2argv-disk-vhost-scsi-num_queues.xml|   29 +
 .../qemuxml2argv-disk-vhost-scsi-wwpn.args |9 ++
 .../qemuxml2argv-disk-vhost-scsi-wwpn.xml  |   29 +
 tests/qemuxml2argvtest.c   |   12 ++
 tests/qemuxml2xmltest.c|4 +
 26 files changed, 399 insertions(+), 23 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-cmd_per_lun.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-cmd_per_lun.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-max_sectors.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-max_sectors.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-num_queues.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-num_queues.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-wwpn.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-wwpn.xml

-- 
1.7.9.5

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


[libvirt] [PATCH 1/2] qemu: add vhost-scsi-pci definitions

2014-07-23 Thread Nicholas A. Bellinger
From: Mike Perez thin...@gmail.com

This patch adds the necessary definitions to support vhost-scsi-pci and
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VHOST_SCSI in QEMU = v1.5.x code.

This includes:

  - Add QEMU_CAPS_VHOST_SCSI
  - Add virDomainControllerDef-wwpn pointer for the vhost-scsi wwpn.
  - Setting the new model type in qemuSetSCSIControllerModel()
  - Invoking virDomainPCIAddressReserveAddr() for vhost-scsi-pci in
qemuAssignDevicePCISlots()
  - Add the 'wwpn=' device parameter in qemuBuildControllerDevStr()
  - Add vhost-scsi-pci to = v1.5.x qemucapabilitiesdata
  - Add xml2argv and xml2xml tests for cmd_per_lun + max_sectors +
num_queues + wwpn.

Signed-off-by: Mike Perez thin...@gmail.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
 Makefile.am|2 +-
 docs/formatdomain.html.in  |   20 +++
 docs/schemas/domaincommon.rng  |5 ++
 src/conf/domain_conf.c |   20 ++-
 src/conf/domain_conf.h |2 +
 src/qemu/qemu_capabilities.c   |2 +
 src/qemu/qemu_capabilities.h   |1 +
 src/qemu/qemu_command.c|   59 ++--
 src/vmx/vmx.c  |1 +
 tests/qemucapabilitiesdata/caps_1.5.3-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |1 +
 .../qemuxml2argv-disk-vhost-scsi-cmd_per_lun.args  |9 +++
 .../qemuxml2argv-disk-vhost-scsi-cmd_per_lun.xml   |   29 ++
 .../qemuxml2argv-disk-vhost-scsi-max_sectors.args  |9 +++
 .../qemuxml2argv-disk-vhost-scsi-max_sectors.xml   |   29 ++
 .../qemuxml2argv-disk-vhost-scsi-num_queues.args   |9 +++
 .../qemuxml2argv-disk-vhost-scsi-num_queues.xml|   29 ++
 .../qemuxml2argv-disk-vhost-scsi-wwpn.args |9 +++
 .../qemuxml2argv-disk-vhost-scsi-wwpn.xml  |   29 ++
 tests/qemuxml2argvtest.c   |   12 
 tests/qemuxml2xmltest.c|4 ++
 22 files changed, 266 insertions(+), 17 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-cmd_per_lun.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-cmd_per_lun.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-max_sectors.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-max_sectors.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-num_queues.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-num_queues.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-wwpn.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-vhost-scsi-wwpn.xml

diff --git a/Makefile.am b/Makefile.am
index a374e1a..cb8d606 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,7 +19,7 @@
 LCOV = lcov
 GENHTML = genhtml
 
-SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \
+SUBDIRS = . gnulib/lib include src daemon tools gnulib/tests \
   tests po examples/object-events examples/hellolibvirt \
   examples/dominfo examples/domsuspend examples/apparmor \
   examples/xml/nwfilter examples/openauth examples/systemtap \
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..32b1864 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2550,16 +2550,16 @@
   control how many devices can be connected through the
   controller.  A scsi controller has an optional
   attribute codemodel/code, which is one of auto, buslogic,
-  ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi or
-  vmpvscsi.  A usb controller has an optional attribute
-  codemodel/code, which is one of piix3-uhci, piix4-uhci, ehci,
-  ich9-ehci1, ich9-uhci1, ich9-uhci2, ich9-uhci3, vt82c686b-uhci,
-  pci-ohci or nec-xhci.  Additionally,
-  span class=sincesince 0.10.0/span, if the USB bus needs to be
-  explicitly disabled for the guest, codemodel='none'/code may be
-  used.  span class=sinceSince 1.0.5/span, no default USB controller
-  will be built on s390.  The PowerPC64 spapr-vio addresses do not have 
an
-  associated controller.
+  ibmvscsi, lsilogic, lsisas1068, lsisas1078, virtio-scsi,
+  vhost-scsi, or vmpvscsi.  A usb controller has an optional
+  attribute codemodel/code, which is one of piix3-uhci, piix4-uhci,
+  ehci, ich9-ehci1, ich9-uhci1, ich9-uhci2, ich9-uhci3,
+  vt82c686b-uhci, pci-ohci or nec-xhci.  Additionally, span
+  class=sincesince 0.10.0/span, if the USB bus needs to be explicitly
+  disabled for the guest, codemodel='none'/code may be used.  span
+  class=sinceSince 1.0.5/span, no default USB controller will be built
+  on s390.  The 

Re: [libvirt] [PATCH] util: virTimeFieldsThenRaw never return negative

2014-07-23 Thread Martin Kletzander

On Thu, Jul 24, 2014 at 10:17:45AM +0800, Wang Rui wrote:

From: James james.wangyu...@huawei.com

virTimeFieldsThenRaw will never return negative result, so I delete related
judgement.



Looking at the code I think it was pretty nicely prepared for error
reporting, but then there was no error that could happen in the
function, so this cleanup makes sense.


Signed-off-by: James james.wangyu...@huawei.com
---
src/util/virtime.c | 7 ++-
src/util/virtime.h | 4 ++--
2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/util/virtime.c b/src/util/virtime.c
index 2a91ea5..662c161 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -121,9 +121,8 @@ const unsigned short int __mon_yday[2][13] = {
 * Converts the timestamp @when into broken-down field format.
 * Time time is always in UTC
 *
- * Returns 0 on success, -1 on error with errno set
 */
-int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
+void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
{
/* This code is taken from GLibC under terms of LGPLv2+ */
long int days, rem, y;
@@ -171,7 +170,6 @@ int virTimeFieldsThenRaw(unsigned long long when, struct tm 
*fields)
days -= ip[y];
fields-tm_mon = y;
fields-tm_mday = days + 1;
-return 0;
}


@@ -209,8 +207,7 @@ int virTimeStringThenRaw(unsigned long long when, char *buf)
{
struct tm fields;

-if (virTimeFieldsThenRaw(when, fields)  0)
-return -1;
+virTimeFieldsThenRaw(when, fields);



This is OK, but there's a virTimeFieldsNowRaw() function that does:
return virTimeFieldsThenRaw(...);

That should be fixed as well.  And virTimeFieldsThen() encapsulates
the Raw version with an error message that's pointless now.  So that
function can be void too.  The whole chain of function calls should be
fixed up, not just one call to it.


fields.tm_year += 1900;
fields.tm_mon += 1;
diff --git a/src/util/virtime.h b/src/util/virtime.h
index 25332db..61f36dc 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -43,12 +43,12 @@ int virTimeMillisNowRaw(unsigned long long *now)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virTimeFieldsNowRaw(struct tm *fields)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
-ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
int virTimeStringNowRaw(char *buf)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virTimeStringThenRaw(unsigned long long when, char *buf)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+void virTimeFieldsThenRaw(unsigned long long when, struct tm *fields)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;



No need to move this; I think it's probably sorted by function name or
it has the same order as the .c file.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list