[libvirt] systemd, LXC and user namespaces

[Richard Weinberger]

Hi!

I'm trying to get rid of a hack to make systemd (kind of) work in
Linux containers on libvirt.
The hack can be found in the first mail of [0].
systemd folks told me that systemd needs a name=systemd cgroup [0],
which makes perfectly sense to me.

I found that libvirt does this already, but uid 0 within the container
is not allowed to access it. (Maybe as Kay noted a chmod() is missing)
Now I'm wondering whether this is simply not supported in libvirt (I'm
on 1.2.1) or am I doing something horrible wrong.

This is my domain:
---cut---
domain type='lxc'
namemy2ndcontainer/name
memory524288/memory
os
typeexe/type
init/bin/bash/init
/os
idmap
!-- here be dragons, the mapping is non-linear --
uid start='0' target='10' count='998'/
gid start='0' target='10' count='998'/
uid start='65533' target='100998' count='2'/
gid start='65533' target='100998' count='2'/
/idmap
devices
console type='pty'/
filesystem type='mount'
source dir='/home/container//my2ndcontainer/rootfs'/
target dir='/'/
/filesystem
interface type='bridge'
source bridge='br0'/
mac address='4a:19:0a:01:01:a4'/
/interface
/devices
/domain
---cut---

Within my domain:

---cut---
test1:/ # mount
/dev/vda2 on / type ext4 (rw,relatime,data=ordered)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
proc on /proc/sys type proc (ro,relatime)
sysfs on /sys type sysfs (ro,relatime)
libvirt on /proc/meminfo type fuse
(rw,nosuid,nodev,relatime,user_id=0,group_id=0,allow_other)
tmpfs on /sys/fs/cgroup type tmpfs
(rw,nosuid,nodev,noexec,relatime,size=64k,mode=755,uid=10,gid=10)
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup
(rw,nosuid,nodev,noexec,relatime,cpuacct,cpu)
cgroup on /sys/fs/cgroup/cpuset type cgroup
(rw,nosuid,nodev,noexec,relatime,cpuset)
cgroup on /sys/fs/cgroup/memory type cgroup
(rw,nosuid,nodev,noexec,relatime,memory)
cgroup on /sys/fs/cgroup/devices type cgroup
(rw,nosuid,nodev,noexec,relatime,devices)
cgroup on /sys/fs/cgroup/freezer type cgroup
(rw,nosuid,nodev,noexec,relatime,freezer)
cgroup on /sys/fs/cgroup/blkio type cgroup
(rw,nosuid,nodev,noexec,relatime,blkio)
cgroup on /sys/fs/cgroup/net_cls type cgroup
(rw,nosuid,nodev,noexec,relatime,net_cls)
cgroup on /sys/fs/cgroup/perf_event type cgroup
(rw,nosuid,nodev,noexec,relatime,perf_event)
cgroup on /sys/fs/cgroup/systemd type cgroup
(rw,nosuid,nodev,noexec,relatime,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd)
devfs on /dev type tmpfs (rw,nosuid,relatime,size=64k,mode=755)
devpts on /dev/pts type devpts (rw,nosuid,relatime,gid=5,mode=620,ptmxmode=666)
devpts on /dev/ptmx type devpts (rw,nosuid,relatime,gid=5,mode=620,ptmxmode=666)
test1:/ # ls -la /sys/fs/cgroup/systemd
total 0
drwxr-xr-x  2 nobody nogroup   0 Feb  6 09:05 .
drwxr-xr-x 11 root   root260 Feb  6 09:05 ..
-rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 cgroup.clone_children
--w--w--w-  1 nobody nogroup   0 Feb  6 09:05 cgroup.event_control
-rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 cgroup.procs
-rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 notify_on_release
-rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 tasks
test1:/ # exec /sbin/init
systemd 208 running in system mode. (+PAM +LIBWRAP +AUDIT +SELINUX
-IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ)
Detected virtualization 'lxc-libvirt'.

Welcome to openSUSE 13.1 (Bottle) (x86_64)!

Set hostname to my2ndcontainer.
Failed to install release agent, ignoring: No such file or directory
Failed to create root cgroup hierarchy: Permission denied
Failed to allocate manager object: Permission denied
---cut---

You can see that systemd stops executing because it was unable to
write to /sys/fs/cgroup/systemd.

Is this a configuration issue or does libvirt need some changes?

[0] 
http://lists.freedesktop.org/archives/systemd-devel/2014-February/016699.html

-- 
Thanks,
//richard

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

Re: [libvirt] [PATCH 2/2] qemu: blockjob: Print correct file name in error message

[Peter Krempa]

On 02/05/14 19:41, Eric Blake wrote:
 On 02/05/2014 10:44 AM, Peter Krempa wrote:
 When attempting a blockcommit from the top layer, the base argument
 passed is NULL. This will be dereferenced when attempting a commit with
 an empty image chain. Output the real volume path instead:

 virsh blockcommit --verbose --path vda --domain DOMNAME --wait
 error: invalid argument: top '/path/somefile' in chain for 'vda' has no 
 backing file

 instead of:

 error: invalid argument: top '(null)' in chain for 'vda' has no backing file
 ---
  src/qemu/qemu_driver.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 ACK
 

Series is now pushed. Thanks.

Peter




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] qemu: Fix crash in virDomainMemoryStats with old qemu

[Jiri Denemark]

On Wed, Feb 05, 2014 at 15:12:03 -0700, Eric Blake wrote:
 On 02/05/2014 07:58 AM, John Ferlan wrote:
  
  
  On 02/05/2014 08:19 AM, Jiri Denemark wrote:
  If virDomainMemoryStats was run on a domain with virtio balloon driver
  running on an old qemu which supports QMP but does not support qom-list
  QMP command, libvirtd would crash. The reason is we did not check if
  qemuMonitorJSONGetObjectListPaths failed and moreover we even stored its
  result in an unsigned integer type.
 
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
 
  Notes:
  version 2:
  - use signed type for i and j to avoid comparison between signed and
unsigned types; gcc-- for not complaining about it
...
 I see no problem with returning -1 if qom-list doesn't exist.  I'm happy
 with the patch as-is:
 
 ACK.

Thanks and pushed.

Jirka

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

Re: [libvirt] [PATCH 2/2] qemuxml2argvtest: Test localtime clock basis

[Roman Bogorodskiy]

  Eric Blake wrote:

 On 02/05/2014 07:32 AM, Michal Privoznik wrote:
  When trying to introduce a test for previous patch, I've
  noticed that the command line is constructed using current
  time. This won't work in our test suite (unless you guys
  wants to set a specific time prior to each test run :) ).
  Therefore we need to mock calls to time(2) to return the
  same value every time it's called.
 
 Slick.  But as we only support mocking calls on Linux, you have
 converted a test from generic platform to Linux-only.  Then again, qemu
 tests only run where we support qemu, which is currently Linux only, so
 I'm not sure it will matter.  I guess we'll find out if anyone complains
 that it broke 'make check' on their platform.

I'm not sure if I have a good understanding of what 'supported' means
here, but at least I can say that qemu driver is 'functional' on FreeBSD
with some known limitations (i.e. missing firewalling part in bridge
driver, etc).

Roman Bogorodskiy

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

Re: [libvirt] [PATCH 2/2] qemuxml2argvtest: Test localtime clock basis

[Daniel P. Berrange]

On Wed, Feb 05, 2014 at 03:26:23PM -0700, Eric Blake wrote:
 On 02/05/2014 07:32 AM, Michal Privoznik wrote:
  When trying to introduce a test for previous patch, I've
  noticed that the command line is constructed using current
  time. This won't work in our test suite (unless you guys
  wants to set a specific time prior to each test run :) ).
  Therefore we need to mock calls to time(2) to return the
  same value every time it's called.
 
 Slick.  But as we only support mocking calls on Linux, you have
 converted a test from generic platform to Linux-only.  Then again, qemu
 tests only run where we support qemu, which is currently Linux only, so
 I'm not sure it will matter.  I guess we'll find out if anyone complains
 that it broke 'make check' on their platform.

FYI as a general rule the LD_PRELOAD approach should be capable
of working on any ELF platform, which is basically everything
we care about except Windows. The cwrap project from samba which
uses the same LD_PRELOAD trick claims to have validated such
portability.

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 v3] bhyve: add a basic driver

[Daniel P. Berrange]

On Thu, Feb 06, 2014 at 12:07:10AM +0400, Roman Bogorodskiy wrote:
   Daniel P. Berrange wrote:
 
   +virCommandAddArg(cmd, -H); /* vmexit from guest on hlt */
   +virCommandAddArg(cmd, -P); /* vmexit from guest on pause */
  
  What's the functional effect of having these set, or not ?
 
 Having that set should make bhyve process terminate on these events (htl
 and pause).

Ok, was just wonder if they needed to be expressed in the
XML or not, but sounds quite low level so lets ignore it
for now.

 I guess it'd be better not to use it so bhyve process doesn't suddenly
 vanish from libvirt radar?

I don't know really - as long as the driver detects that it has gone
and marks the guest as shutdown.

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] systemd, LXC and user namespaces

[Daniel P. Berrange]

On Thu, Feb 06, 2014 at 10:36:09AM +0100, Richard Weinberger wrote:
 Hi!
 
 I'm trying to get rid of a hack to make systemd (kind of) work in
 Linux containers on libvirt.
 The hack can be found in the first mail of [0].
 systemd folks told me that systemd needs a name=systemd cgroup [0],
 which makes perfectly sense to me.
 
 I found that libvirt does this already, but uid 0 within the container
 is not allowed to access it. (Maybe as Kay noted a chmod() is missing)
 Now I'm wondering whether this is simply not supported in libvirt (I'm
 on 1.2.1) or am I doing something horrible wrong.

The configuration looks fine, provided that you have ensured
that your files in /home/container/my2ndcontainer/rootfs have
been chown'd to match the target UID/GID values you've setup
Libvirt doesn't do chowning of any filesystems you provide,
only things it creates.

 This is my domain:
 ---cut---
 domain type='lxc'
 namemy2ndcontainer/name
 memory524288/memory
 os
 typeexe/type
 init/bin/bash/init
 /os
 idmap
 !-- here be dragons, the mapping is non-linear --
 uid start='0' target='10' count='998'/
 gid start='0' target='10' count='998'/
 uid start='65533' target='100998' count='2'/
 gid start='65533' target='100998' count='2'/
 /idmap
 devices
 console type='pty'/
 filesystem type='mount'
 source dir='/home/container//my2ndcontainer/rootfs'/
 target dir='/'/
 /filesystem
 interface type='bridge'
 source bridge='br0'/
 mac address='4a:19:0a:01:01:a4'/
 /interface
 /devices
 /domain
 ---cut---
 
 Within my domain:

 test1:/ # ls -la /sys/fs/cgroup/systemd
 total 0
 drwxr-xr-x  2 nobody nogroup   0 Feb  6 09:05 .
 drwxr-xr-x 11 root   root260 Feb  6 09:05 ..
 -rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 cgroup.clone_children
 --w--w--w-  1 nobody nogroup   0 Feb  6 09:05 cgroup.event_control
 -rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 cgroup.procs
 -rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 notify_on_release
 -rw-r--r--  1 nobody nogroup   0 Feb  6 09:05 tasks

Ok, so this seems to confirm the guess I had in my response to your
mail on systemd-devel.  Libvirt appears to have forgotten to chown
the cgroups directory to provide access to systemd. Hence the system
is remapping it to the overflow uid/gid

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] [PATCHv3 09/10] Fix misspelled cpuacct.usage_percpu in cgroup mock.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

---

Turned out during test writing, when this was actually used the
first time.

  tests/vircgroupmock.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index d772652..84bb1b0 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -214,7 +214,7 @@ static int make_controller(const char *path, mode_t mode)
user 216687025\n
system 43421396\n);
  MAKE_FILE(cpuacct.usage, 2787788855799582\n);
-MAKE_FILE(cpuacct.usage_per_cpu, 1413142688153030 
1374646168910542\n);
+MAKE_FILE(cpuacct.usage_percpu, 1413142688153030 
1374646168910542\n);
  } else if (STRPREFIX(controller, cpuset)) {
  MAKE_FILE(cpuset.cpu_exclusive, 1\n);
  if (STREQ(controller, cpuset))



ACK

Michal

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

Re: [libvirt] [PATCHv3 05/10] Implement lxcDomainBlockStats* for lxc driver

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions.
---

Notes v3:
  - merged patch, both api methods now added in one
  - addressed comments from v2 review
  - check for cgroup controllers actually being mounted

  src/lxc/lxc_driver.c | 196 +++
  1 file changed, 196 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 39cd981..103352c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -75,6 +75,7 @@


  #define LXC_NB_MEM_PARAM  3
+#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4


  static int lxcStateInitialize(bool privileged,
@@ -2202,6 +2203,199 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,


  static int
+lxcDomainBlockStats(virDomainPtr dom,
+const char *path,
+struct _virDomainBlockStats *stats)
+{
+int ret = -1, idx;
+virDomainObjPtr vm;
+virDomainDiskDefPtr disk = NULL;
+virLXCDomainObjPrivatePtr priv;
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return ret;
+
+priv = vm-privateData;
+
+if (virDomainBlockStatsEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(domain is not running));
+goto cleanup;
+}
+
+if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(blkio cgroup isn't mounted));
+goto cleanup;
+}
+
+if (!*path) {
+/* empty path - return entire domain blkstats instead */
+ret = virCgroupGetBlkioIoServiced(priv-cgroup,
+  stats-rd_bytes,
+  stats-wr_bytes,
+  stats-rd_req,
+  stats-wr_req);
+goto cleanup;
+}


This is interesting. In the qemu driver we don't allow this, and neither 
do we in the public API (well, at least in API documentation), nor 
virsh. I'm okay with this as long as we relax these restrictions in the 
places mentioned earlier.



+
+if ((idx = virDomainDiskIndexByName(vm-def, path, false))  0) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(invalid path: %s), path);
+goto cleanup;
+}
+disk = vm-def-disks[idx];
+
+if (!disk-info.alias) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(missing disk device alias name for %s), disk-dst);
+goto cleanup;
+}
+
+ret = virCgroupGetBlkioIoDeviceServiced(priv-cgroup,
+disk-info.alias,
+stats-rd_bytes,
+stats-wr_bytes,
+stats-rd_req,
+stats-wr_req);
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+
+static int
+lxcDomainBlockStatsFlags(virDomainPtr dom,
+ const char * path,
+ virTypedParameterPtr params,
+ int * nparams,
+ unsigned int flags)
+{
+int tmp, ret = -1, idx;
+virDomainObjPtr vm;
+virDomainDiskDefPtr disk = NULL;
+virLXCDomainObjPrivatePtr priv;
+long long rd_req, rd_bytes, wr_req, wr_bytes;
+virTypedParameterPtr param;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+/* We don't return strings, and thus trivially support this flag.  */
+flags = ~VIR_TYPED_PARAM_STRING_OKAY;
+
+if (!params  !*nparams) {
+*nparams = LXC_NB_DOMAIN_BLOCK_STAT_PARAM;
+return 0;
+}
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return ret;
+
+priv = vm-privateData;
+
+if (virDomainBlockStatsFlagsEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(domain is not running));
+goto cleanup;
+}
+
+if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(blkio cgroup isn't mounted));
+goto cleanup;
+}
+
+if (!*path) {
+/* empty path - return entire domain blkstats instead */
+if (virCgroupGetBlkioIoServiced(priv-cgroup,
+rd_bytes,
+wr_bytes,
+rd_req,
+wr_req)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(domain stats query failed));
+goto cleanup;
+}
+}
+else {


We tend to write this as '} 

Re: [libvirt] [PATCHv3 00/10] Add BlkIO and CPU/mem stat API implementations for lxc

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

This patch set adds block io, memory and domain cpu statistics API
slot implementations to the LXC driver, in order to get linux
container monitoring and accounting a bit closer to qemu standards.

The last patch is a tad quirky (happy to hear suggestions on
alternative ways), in that it widens the permissible value set
at the .domainBlockStats slot: for lxc guests, it is relatively
likely to have zero disk devices, since host filesystems can be
used via passthrough bind mounts. Therefore, passing the zero-length
string as device path, is interpreted as 'return summary stats for
the entire domains's block io'.


Right. We can add test for both, can't we?

Anyway, I've ACKed most of the patches except a few. Can you please fix 
 resend them? I'll do the review and (hopefully) push them too. Nice work!


Michal

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

Re: [libvirt] [PATCHv3 08/10] Add unit test for virCgroupGetMemoryUsage.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

---
  tests/vircgrouptest.c | 35 +++
  1 file changed, 35 insertions(+)

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index a29cdd2..6826442 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -532,6 +532,38 @@ static int testCgroupAvailable(const void *args)
  return 0;
  }

+static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
+{
+virCgroupPtr cgroup = NULL;
+int rv, ret = -1;
+unsigned long kb;
+
+if ((rv = virCgroupNewPartition(/virtualmachines, true,
+(1  VIR_CGROUP_CONTROLLER_MEMORY),
+cgroup))  0) {
+fprintf(stderr, Could not create /virtualmachines cgroup: %d\n, -rv);
+goto cleanup;
+}
+
+if ((rv = virCgroupGetMemoryUsage(cgroup, kb))  0) {
+fprintf(stderr, Could not retrieve GetMemoryUsage for /virtualmachines 
cgroup: %d\n, -rv);
+goto cleanup;
+}
+
+if (kb != 1421212UL) {
+fprintf(stderr,
+Wrong value from virCgroupGetMemoryUsage (expected %ld)\n,
+1421212UL);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+virCgroupFree(cgroup);
+return ret;
+}
+
  static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED)
  {
  virCgroupPtr cgroup = NULL;
@@ -704,6 +736,9 @@ mymain(void)
  if (virtTestRun(virCgroupGetBlkioIoDeviceServiced works, 
testCgroupGetBlkioIoDeviceServiced, NULL)  0)
  ret = -1;

+if (virtTestRun(virCgroupGetMemoryUsage works, testCgroupGetMemoryUsage, 
NULL)  0)
+ret = -1;
+
  setenv(VIR_CGROUP_MOCK_MODE, allinone, 1);
  if (virtTestRun(New cgroup for self (allinone), 
testCgroupNewForSelfAllInOne, NULL)  0)
  ret = -1;



ACK

Michal

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

Re: [libvirt] [PATCHv3 10/10] Add unit test for virCgroupGetPercpuStats.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

---
  tests/vircgrouptest.c | 66 +++
  1 file changed, 66 insertions(+)

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 6826442..dfcb0aa 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -32,6 +32,7 @@
  # include virerror.h
  # include virlog.h
  # include virfile.h
+# include nodeinfo.h

  # define VIR_FROM_THIS VIR_FROM_NONE

@@ -532,6 +533,68 @@ static int testCgroupAvailable(const void *args)
  return 0;
  }

+static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
+{
+virCgroupPtr cgroup = NULL;
+size_t i;
+int rv, ret = -1;
+virTypedParameter params[2];
+
+// TODO: mock nodeGetCPUCount() as well  check 2nd cpu, too
+unsigned long long expected[] = {
+1413142688153030
+};
+
+if ((rv = virCgroupNewPartition(/virtualmachines, true,
+(1  VIR_CGROUP_CONTROLLER_CPU) |
+(1  VIR_CGROUP_CONTROLLER_CPUACCT),
+cgroup))  0) {
+fprintf(stderr, Could not create /virtualmachines cgroup: %d\n, -rv);
+goto cleanup;
+}
+
+if (nodeGetCPUCount()  1) {
+fprintf(stderr, Unexpected: nodeGetCPUCount() yields: %d\n, 
nodeGetCPUCount());
+goto cleanup;
+}
+
+if ((rv = virCgroupGetPercpuStats(cgroup,
+  params,
+  2, 0, 1))  0) {
+fprintf(stderr, Failed call to virCgroupGetPercpuStats for 
/virtualmachines cgroup: %d\n, -rv);
+goto cleanup;
+}
+
+for (i = 0; i  ARRAY_CARDINALITY(expected); i++) {
+if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) {
+fprintf(stderr,
+Wrong parameter name value from virCgroupGetPercpuStats (is: 
%s)\n,
+params[i].field);
+goto cleanup;
+}
+
+if (params[i].type != VIR_TYPED_PARAM_ULLONG) {
+fprintf(stderr,
+Wrong parameter value type from virCgroupGetPercpuStats (is: 
%d)\n,
+params[i].type);
+goto cleanup;
+}
+
+if (params[i].value.ul != expected[i]) {
+fprintf(stderr,
+Wrong value from virCgroupGetMemoryUsage (expected 
%llu)\n,
+params[i].value.ul);
+goto cleanup;
+}
+}
+
+ret = 0;
+
+cleanup:
+virCgroupFree(cgroup);
+return ret;
+}
+
  static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
  {
  virCgroupPtr cgroup = NULL;
@@ -739,6 +802,9 @@ mymain(void)
  if (virtTestRun(virCgroupGetMemoryUsage works, testCgroupGetMemoryUsage, 
NULL)  0)
  ret = -1;

+if (virtTestRun(virCgroupGetPercpuStats works, testCgroupGetPercpuStats, 
NULL)  0)
+ret = -1;
+
  setenv(VIR_CGROUP_MOCK_MODE, allinone, 1);
  if (virtTestRun(New cgroup for self (allinone), 
testCgroupNewForSelfAllInOne, NULL)  0)
  ret = -1;



ACK

Michal

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

Re: [libvirt] [PATCHv3 07/10] Add unit test for virCgroupGetBlkioIo*Serviced

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:
 ---
 
 Adds another dummy device to push the cgroup parsing code a bit
 harder
 
   tests/vircgroupmock.c | 107 +++-
   tests/vircgrouptest.c | 133 
 ++
   2 files changed, 238 insertions(+), 2 deletions(-)
 
 diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
 index 6542973..d772652 100644
 --- a/tests/vircgroupmock.c
 +++ b/tests/vircgroupmock.c
 @@ -34,6 +34,8 @@
   static int (*realopen)(const char *path, int flags, ...);
   static FILE *(*realfopen)(const char *path, const char *mode);
   static int (*realaccess)(const char *path, int mode);
 +static int (*realstat)(const char *path, struct stat *sb);
 +static int (*real__xstat)(int ver, const char *path, struct stat *sb);
   static int (*reallstat)(const char *path, struct stat *sb);
   static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
   static int (*realmkdir)(const char *path, mode_t mode);
 @@ -43,6 +45,8 @@ static int (*realmkdir)(const char *path, mode_t mode);
* vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in 
 an inline function with external linkage [-Werror,-Wstatic-in-inline]
*/
   char *fakesysfsdir;
 +char *fakedevicedir0;
 +char *fakedevicedir1;
   
   
   # define SYSFS_PREFIX /not/really/sys/fs/cgroup/
 @@ -332,13 +336,23 @@ static int make_controller(const char *path, mode_t 
 mode)
 8:0 Write 411440480256\n
 8:0 Sync 248486822912\n
 8:0 Async 222495764480\n
 -  8:0 Total 470982587392\n);
 +  8:0 Total 470982587392\n
 +  9:0 Read 59542107137\n
 +  9:0 Write 411440480257\n
 +  9:0 Sync 248486822912\n
 +  9:0 Async 222495764480\n
 +  9:0 Total 470982587392\n);
   MAKE_FILE(blkio.throttle.io_serviced,
 8:0 Read 4832583\n
 8:0 Write 36641903\n
 8:0 Sync 30723171\n
 8:0 Async 10751315\n
 -  8:0 Total 41474486\n);
 +  8:0 Total 41474486\n
 +  9:0 Read 4832584\n
 +  9:0 Write 36641904\n
 +  9:0 Sync 30723171\n
 +  9:0 Async 10751315\n
 +  9:0 Total 41474486\n);
   MAKE_FILE(blkio.throttle.read_bps_device, );
   MAKE_FILE(blkio.throttle.read_iops_device, );
   MAKE_FILE(blkio.throttle.write_bps_device, );
 @@ -382,6 +396,7 @@ static void init_syms(void)
   LOAD_SYM(fopen);
   LOAD_SYM(access);
   LOAD_SYM_ALT(lstat, __lxstat);
 +LOAD_SYM_ALT(stat, __xstat);
   LOAD_SYM(mkdir);
   LOAD_SYM(open);
   }
 @@ -396,6 +411,16 @@ static void init_sysfs(void)
   abort();
   }
   
 +if (!(fakedevicedir0 = getenv(LIBVIRT_FAKE_DEVICE_DIR0))) {
 +fprintf(stderr, Missing LIBVIRT_FAKE_DEVICE_DIR0 env variable\n);
 +abort();
 +}
 +
 +if (!(fakedevicedir1 = getenv(LIBVIRT_FAKE_DEVICE_DIR1))) {
 +fprintf(stderr, Missing LIBVIRT_FAKE_DEVICE_DIR1 env variable\n);
 +abort();
 +}
 +
   # define MAKE_CONTROLLER(subpath)   \
   do {\
   char *path; \
 @@ -529,6 +554,14 @@ int __lxstat(int ver, const char *path, struct stat *sb)
   }
   ret = real__lxstat(ver, newpath, sb);
   free(newpath);
 +} else if (STRPREFIX(path, fakedevicedir0)) {
 +sb-st_mode = S_IFBLK;
 +sb-st_rdev = makedev(8, 0);
 +return 0;
 +} else if (STRPREFIX(path, fakedevicedir1)) {
 +sb-st_mode = S_IFBLK;
 +sb-st_rdev = makedev(9, 0);
 +return 0;
   } else {
   ret = real__lxstat(ver, path, sb);
   }

This won't work. If one __lxstat, lstat, __xstat, stat is called over path that 
does not start with SYSFS_PREFIX, in fact it has not been called with such 
prefix yet, then the fakedevicedir0 and fakedevicedir1 are NULL and hence 
strncmp will SIGSEGV:

Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2_pminub () at ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S:38
38  ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S: No such file or 
directory.
(gdb) bt
#0  __strlen_sse2_pminub () at 
../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S:38
#1  0x77df8f79 in __xstat (ver=1, path=0x6215a0 /etc/libnl/classid, 
sb=0x7fffd7e0) at vircgroupmock.c:619
#2  0x003468625627 in rtnl_tc_read_classid_file () from 
/usr/lib64/libnl-route-3.so.200
#3  0x003468617e99 in ?? () from /usr/lib64/libnl-route-3.so.200
#4  0x00345e80ec2a in call_init (l=optimized out, argc=1, 
argv=0x7fffda38, env=0x7fffda48) at dl-init.c:78
#5  0x00345e80ecfc in _dl_init 

Re: [libvirt] [PATCHv3 06/10] Widening API change - accept empty path for virDomainBlockStats

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

And provide domain summary stat in that case, for lxc backend.
Use case is a container inheriting all devices from the host,
e.g. when doing application containerization.
---
  src/libvirt.c|  8 ++--
  tools/virsh-domain-monitor.c | 11 ---
  tools/virsh.pod  |  5 +++--
  3 files changed, 17 insertions(+), 7 deletions(-)



Aah, I didn't see this coming :)


diff --git a/src/libvirt.c b/src/libvirt.c
index 9cc5b1c..64da438 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -7747,7 +7747,9 @@ error:
   * an unambiguous source name of the block device (the source
   * file='...'/ sub-element, such as /path/to/image).  Valid names
   * can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
   *
   * Domains may have more than one block device.  To get stats for
   * each you should make multiple calls to this function.
@@ -7813,7 +7815,9 @@ error:
   * an unambiguous source name of the block device (the source
   * file='...'/ sub-element, such as /path/to/image).  Valid names
   * can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
   *
   * Domains may have more than one block device.  To get stats for
   * each you should make multiple calls to this function.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index de4afbb..105f841 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -888,7 +888,7 @@ static const vshCmdOptDef opts_domblkstat[] = {
  },
  {.name = device,
   .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .flags = VSH_OFLAG_EMPTY_OK,
   .help = N_(block device)
  },
  {.name = human,
@@ -954,8 +954,13 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
  if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
  return false;

-if (vshCommandOptStringReq(ctl, cmd, device, device)  0)
-goto cleanup;
+/* device argument is optional now. if it's missing, supply empty
+   string to denote 'all devices'. A NULL device arg would violate
+   API contract.
+ */
+rc = vshCommandOptStringReq(ctl, cmd, device, device); /* and ignore rc 
*/
+if (!device)
+device = ;

  rc = virDomainBlockStatsFlags(dom, device, NULL, nparams, 0);

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9e670da..3e45177 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -623,12 +623,13 @@ If I--graceful is specified, don't resort to extreme 
measures
  (e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout;
  return an error instead.

-=item Bdomblkstat Idomain Iblock-device [I--human]
+=item Bdomblkstat Idomain [Iblock-device] [I--human]

  Get device block stats for a running domain.  A Iblock-device corresponds
  to a unique target name (target dev='name'/) or source file (source
  file='name'/) for one of the disk devices attached to Idomain (see
-also Bdomblklist for listing these names).
+also Bdomblklist for listing these names). On a lxc domain, omitting the
+Iblock-device yields device block stats summarily for the entire domain.

  Use I--human for a more human readable output.




ACK to this one and the previous one too then.

Michal

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

Re: [libvirt] [PATCHv3 04/10] Implement domainGetCPUStats for lxc driver.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

---

Notes v3:
  - moved most of the code out to virCgroupGetPercpuStats, for better
testability.
  - addressed comments from v2 review

  src/libvirt_private.syms |  1 +
  src/lxc/lxc_driver.c | 51 
  src/util/vircgroup.c | 75 
  src/util/vircgroup.h |  7 +
  4 files changed, 134 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 890d6c7..4a7201e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1023,6 +1023,7 @@ virCgroupGetMemorySoftLimit;
  virCgroupGetMemoryUsage;
  virCgroupGetMemSwapHardLimit;
  virCgroupGetMemSwapUsage;
+virCgroupGetPercpuStats;
  virCgroupHasController;
  virCgroupIsolateMount;
  virCgroupKill;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 02b5cc3..39cd981 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -76,6 +76,7 @@

  #define LXC_NB_MEM_PARAM  3

+
  static int lxcStateInitialize(bool privileged,
virStateInhibitCallback callback,
void *opaque);
@@ -5388,6 +5389,55 @@ cleanup:
  }


+static int
+lxcDomainGetCPUStats(virDomainPtr dom,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus,
+ unsigned int flags)
+{
+virDomainObjPtr vm = NULL;
+int ret = -1;
+bool isActive;


This variable is unnecessary. Although I see you are just copying 
structure we have in the qemu driver.



+virLXCDomainObjPrivatePtr priv;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return ret;
+
+priv = vm-privateData;
+
+if (virDomainGetCPUStatsEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+isActive = virDomainObjIsActive(vm);
+if (!isActive) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(domain is not running));
+goto cleanup;
+}
+
+if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(cgroup CPUACCT controller is not mounted));
+goto cleanup;
+}
+
+if (start_cpu == -1)
+ret = virCgroupGetDomainTotalCpuStats(priv-cgroup,
+  params, nparams);
+else
+ret = virCgroupGetPercpuStats(priv-cgroup, params,
+  nparams, start_cpu, ncpus);
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+
  /* Function Tables */
  static virDriver lxcDriver = {
  .no = VIR_DRV_LXC,
@@ -5466,6 +5516,7 @@ static virDriver lxcDriver = {
  .nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */
  .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
  .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
+.domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */
  .nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */
  .nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */
  .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 7427a21..268a4ae 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -53,11 +53,14 @@
  #include virsystemd.h
  #include virtypedparam.h

+#include nodeinfo.h
+
  #define CGROUP_MAX_VAL 512

  #define VIR_FROM_THIS VIR_FROM_CGROUP

  #define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3
+#define CGROUP_NB_PER_CPU_STAT_PARAM   1

  #if defined(__linux__)  defined(HAVE_GETMNTENT_R)  \
  defined(_DIRENT_HAVE_D_TYPE)  defined(_SC_CLK_TCK)
@@ -2824,6 +2827,78 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char 
*path, int perms)
  }


+int
+virCgroupGetPercpuStats(virCgroupPtr group,
+virTypedParameterPtr params,
+unsigned int nparams,
+int start_cpu,
+unsigned int ncpus)
+{
+int rv = -1;
+size_t i;
+int id, max_id;
+char *pos;
+char *buf = NULL;
+virTypedParameterPtr ent;
+int param_idx;
+unsigned long long cpu_time;
+
+/* return the number of supported params */
+if (nparams == 0  ncpus != 0)
+return CGROUP_NB_PER_CPU_STAT_PARAM;
+
+/* To parse account file, we need to know how many cpus are present.  */
+max_id = nodeGetCPUCount();
+if (max_id  0)
+return rv;
+
+if (ncpus == 0) { /* returns max cpu ID */
+rv = max_id;
+goto cleanup;
+}
+
+if (start_cpu  max_id) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(start_cpu %d larger than maximum of %d),
+   start_cpu, max_id);
+goto cleanup;
+}
+

Re: [libvirt] [PATCHv3 02/10] Implement domainMemoryStats API slot for LXC driver.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

---
  src/lxc/lxc_driver.c | 50 ++
  1 file changed, 50 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 138c706..02b5cc3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5169,6 +5169,55 @@ lxcNodeGetInfo(virConnectPtr conn,


  static int
+lxcDomainMemoryStats(virDomainPtr dom,
+ struct _virDomainMemoryStat *stats,
+ unsigned int nr_stats,
+ unsigned int flags)
+{
+virDomainObjPtr vm;
+int ret = -1;
+virLXCDomainObjPrivatePtr priv;
+
+virCheckFlags(0, -1);
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+goto cleanup;
+
+priv = vm-privateData;
+
+if (virDomainMemoryStatsEnsureACL(dom-conn, vm-def)  0)
+goto cleanup;
+
+ret = 0;
+if (!virDomainObjIsActive(vm))
+goto cleanup;
+
+if (ret  nr_stats) {
+stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON;
+stats[ret].val = vm-def-mem.cur_balloon;
+ret++;
+}
+if (ret  nr_stats) {
+stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN;
+virCgroupGetMemSwapUsage(priv-cgroup, stats[ret].val);
+ret++;
+}
+if (ret  nr_stats) {
+unsigned long kb;
+stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
+virCgroupGetMemoryUsage(priv-cgroup, kb);
+stats[ret].val = kb;
+ret++;
+}


Both these virCgroupGetMem* may fail, in which case the return pointer 
is untouched hence we will return bogus value.


Just a side note - all of our virCgroupGet* handling functions take 
unsigned long long (if they take integer at all). Except the 
virCgroupGetMemoryUsage(). I wonder why. In fact quick git grep over the 
function reveals another places where a UL local variable is introduced 
just so it can be later casted to ULL .. If you decide to do something 
with this, it can be a follow up/separate patch to this series.


Michal

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

Re: [libvirt] [PATCHv3 01/10] Add util virCgroupGetBlkioIo*Serviced methods.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

This reads blkio stats from blkio.throttle.io_service_bytes and
blkio.throttle.io_serviced.
---

Notes v3:
  - beyond the minor nits from the last review,
virCgroupGetBlkioIoDeviceServiced was rather busted wrt. string
ptr p. Now fixed, and with unit test (separate patch)

  src/libvirt_private.syms |   2 +
  src/util/vircgroup.c | 254 +++
  src/util/vircgroup.h |  12 +++
  3 files changed, 268 insertions(+)


ACK

Michal

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

Re: [libvirt] [PATCHv3 03/10] Make qemuGetDomainTotalCPUStats a virCgroup function.

[Michal Privoznik]

On 03.02.2014 18:44, Thorsten Behrens wrote:

To reuse this from other drivers, like lxc.
---
  src/libvirt_private.syms |  1 +
  src/qemu/qemu_driver.c   | 54 ++--
  src/util/vircgroup.c | 53 +++
  src/util/vircgroup.h |  5 +
  4 files changed, 61 insertions(+), 52 deletions(-)



ACK, pure code movement.

Michal

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

[libvirt] [PATCH v1.1.0-maint] Push nwfilter update locking up to top level

[Laine Stump]

From: Daniel P. Berrange berra...@redhat.com

(see notes about conflict resolution at the end. I'm not sending the
other 5 patches required, since they were all simple cherry-picks of:

4f2094346d98f4ed6a2de115d204c166cc563496
b77b16ce4166dcc87963ae5d279b77b162ddbb55
ebca369e3fe5ac999c261c2d44e60a1bac3cfe65
999d72fbd59ea712128ae294b69b6a54039d757b
c065984b58000a44c90588198d222a314ac532fd
)

The NWFilter code has as a deadlock race condition between
the virNWFilter{Define,Undefine} APIs and starting of guest
VMs due to mis-matched lock ordering.

In the virNWFilter{Define,Undefine} codepaths the lock ordering
is

  1. nwfilter driver lock
  2. virt driver lock
  3. nwfilter update lock
  4. domain object lock

In the VM guest startup paths the lock ordering is

  1. virt driver lock
  2. domain object lock
  3. nwfilter update lock

As can be seen the domain object and nwfilter update locks are
not acquired in a consistent order.

The fix used is to push the nwfilter update lock upto the top
level resulting in a lock ordering for virNWFilter{Define,Undefine}
of

  1. nwfilter driver lock
  2. nwfilter update lock
  3. virt driver lock
  4. domain object lock

and VM start using

  1. nwfilter update lock
  2. virt driver lock
  3. domain object lock

This has the effect of serializing VM startup once again, even if
no nwfilters are applied to the guest. There is also the possibility
of deadlock due to a call graph loop via virNWFilterInstantiate
and virNWFilterInstantiateFilterLate.

These two problems mean the lock must be turned into a read/write
lock instead of a plain mutex at the same time. The lock is used to
serialize changes to the driver-nwfilters hash, so the write lock
only needs to be held by the define/undefine methods. All other
methods can rely on a read lock which allows good concurrency.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
(cherry picked from commit 6e5c79a1b5a8b3a23e7df7ffe58fb272aa17fbfb)

Conflicts:
src/conf/nwfilter_conf.c
  - virReportOOMError() in context of one hunk.
src/lxc/lxc_driver.c
  - functions renamed, and lxc object locking changed, creating
a conflict in the context.
---
 src/conf/nwfilter_conf.c   | 25 -
 src/conf/nwfilter_conf.h   |  3 ++-
 src/libvirt_private.syms   |  3 ++-
 src/lxc/lxc_driver.c   |  8 +++-
 src/nwfilter/nwfilter_driver.c | 10 ++
 src/nwfilter/nwfilter_gentech_driver.c |  6 +-
 src/qemu/qemu_driver.c |  6 ++
 src/uml/uml_driver.c   |  4 
 8 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 94b4fe9..e943bab 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2,7 +2,7 @@
  * nwfilter_conf.c: network filter XML processing
  *  (derived from storage_conf.c)
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2014 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * Copyright (C) 2010-2011 IBM Corporation
@@ -143,17 +143,22 @@ static const struct int_map chain_priorities[] = {
 /*
  * only one filter update allowed
  */
-static virMutex updateMutex;
+static virRWLock updateLock;
 static bool initialized = false;
 
 void
-virNWFilterLockFilterUpdates(void) {
-virMutexLock(updateMutex);
+virNWFilterReadLockFilterUpdates(void) {
+virRWLockRead(updateLock);
+}
+
+void
+virNWFilterWriteLockFilterUpdates(void) {
+virRWLockWrite(updateLock);
 }
 
 void
 virNWFilterUnlockFilterUpdates(void) {
-virMutexUnlock(updateMutex);
+virRWLockUnlock(updateLock);
 }
 
 
@@ -2992,14 +2997,12 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 return NULL;
 }
 
-virNWFilterLockFilterUpdates();
 
 if ((nwfilter = virNWFilterObjFindByName(nwfilters, def-name))) {
 
 if (virNWFilterDefEqual(def, nwfilter-def, false)) {
 virNWFilterDefFree(nwfilter-def);
 nwfilter-def = def;
-virNWFilterUnlockFilterUpdates();
 return nwfilter;
 }
 
@@ -3007,7 +3010,6 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 /* trigger the update on VMs referencing the filter */
 if (virNWFilterTriggerVMFilterRebuild()) {
 nwfilter-newDef = NULL;
-virNWFilterUnlockFilterUpdates();
 virNWFilterObjUnlock(nwfilter);
 return NULL;
 }
@@ -3015,12 +3017,9 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 virNWFilterDefFree(nwfilter-def);
 nwfilter-def = def;
 nwfilter-newDef = NULL;
-virNWFilterUnlockFilterUpdates();
 return nwfilter;
 }
 
-virNWFilterUnlockFilterUpdates();
-
 if (VIR_ALLOC(nwfilter)  0) {
 virReportOOMError();
 return NULL;
@@ -3492,7 +3491,7 @@ int 

Re: [libvirt] [PATCH] virsh: only report filled values in nodecpustats

[Michal Privoznik]

On 04.02.2014 10:22, Ján Tomko wrote:

Rewrite the function to use an array instead of a struct,
translating the field names to int via an enum.
---
  tools/virsh-host.c | 126 +++--
  1 file changed, 64 insertions(+), 62 deletions(-)



ACK, I like this one more.

Michal

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

Re: [libvirt] [PATCH v1.1.0-maint] Push nwfilter update locking up to top level

[Daniel P. Berrange]

On Thu, Feb 06, 2014 at 02:11:26PM +0200, Laine Stump wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 (see notes about conflict resolution at the end. I'm not sending the
 other 5 patches required, since they were all simple cherry-picks of:
 
 4f2094346d98f4ed6a2de115d204c166cc563496
 b77b16ce4166dcc87963ae5d279b77b162ddbb55
 ebca369e3fe5ac999c261c2d44e60a1bac3cfe65
 999d72fbd59ea712128ae294b69b6a54039d757b
 c065984b58000a44c90588198d222a314ac532fd
 )
 
 The NWFilter code has as a deadlock race condition between
 the virNWFilter{Define,Undefine} APIs and starting of guest
 VMs due to mis-matched lock ordering.
 
 In the virNWFilter{Define,Undefine} codepaths the lock ordering
 is
 
   1. nwfilter driver lock
   2. virt driver lock
   3. nwfilter update lock
   4. domain object lock
 
 In the VM guest startup paths the lock ordering is
 
   1. virt driver lock
   2. domain object lock
   3. nwfilter update lock
 
 As can be seen the domain object and nwfilter update locks are
 not acquired in a consistent order.
 
 The fix used is to push the nwfilter update lock upto the top
 level resulting in a lock ordering for virNWFilter{Define,Undefine}
 of
 
   1. nwfilter driver lock
   2. nwfilter update lock
   3. virt driver lock
   4. domain object lock
 
 and VM start using
 
   1. nwfilter update lock
   2. virt driver lock
   3. domain object lock
 
 This has the effect of serializing VM startup once again, even if
 no nwfilters are applied to the guest. There is also the possibility
 of deadlock due to a call graph loop via virNWFilterInstantiate
 and virNWFilterInstantiateFilterLate.
 
 These two problems mean the lock must be turned into a read/write
 lock instead of a plain mutex at the same time. The lock is used to
 serialize changes to the driver-nwfilters hash, so the write lock
 only needs to be held by the define/undefine methods. All other
 methods can rely on a read lock which allows good concurrency.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 (cherry picked from commit 6e5c79a1b5a8b3a23e7df7ffe58fb272aa17fbfb)
 
 Conflicts:
   src/conf/nwfilter_conf.c
   - virReportOOMError() in context of one hunk.
   src/lxc/lxc_driver.c
   - functions renamed, and lxc object locking changed, creating
 a conflict in the context.
 ---
  src/conf/nwfilter_conf.c   | 25 -
  src/conf/nwfilter_conf.h   |  3 ++-
  src/libvirt_private.syms   |  3 ++-
  src/lxc/lxc_driver.c   |  8 +++-
  src/nwfilter/nwfilter_driver.c | 10 ++
  src/nwfilter/nwfilter_gentech_driver.c |  6 +-
  src/qemu/qemu_driver.c |  6 ++
  src/uml/uml_driver.c   |  4 
  8 files changed, 40 insertions(+), 25 deletions(-)

ACK, you got the lock ordering correct when resolving the
lxc conflict.

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] Looking for project ideas and mentors for Google Summer of Code 2014

[Stefan Hajnoczi]

On Tue, Feb 4, 2014 at 1:38 PM, Michal Privoznik mpriv...@redhat.com wrote:
 On 03.02.2014 08:45, Stefan Hajnoczi wrote:

 KVM  libvirt: you are welcome to join the QEMU umbrella organization
 like last year.


 I've updated wiki with a libvirt idea. But I can sense more to come later as
 I have some time to think about it :)

Great, thanks!

I have added my QEMU block layer ideas too.

Deadline for the organization application is 14th of February.  As
part of the application form we need to show our list of ideas.
Thanks for posting your project ideas so that our list is ready.

Stefan

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

Re: [libvirt] Looking for project ideas and mentors for Google Summer of Code 2014

[Giorgio Zoppi]

Hi all,
i offer part of my free time as mentor.
Cheers,
Giorgio


2014-02-06 13:27 GMT+01:00 Stefan Hajnoczi stefa...@gmail.com:

 On Tue, Feb 4, 2014 at 1:38 PM, Michal Privoznik mpriv...@redhat.com
 wrote:
  On 03.02.2014 08:45, Stefan Hajnoczi wrote:
 
  KVM  libvirt: you are welcome to join the QEMU umbrella organization
  like last year.
 
 
  I've updated wiki with a libvirt idea. But I can sense more to come
 later as
  I have some time to think about it :)

 Great, thanks!

 I have added my QEMU block layer ideas too.

 Deadline for the organization application is 14th of February.  As
 part of the application form we need to show our list of ideas.
 Thanks for posting your project ideas so that our list is ready.

 Stefan

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




-- 
Quiero ser el rayo de sol que cada día te despierta
para hacerte respirar y vivir en me.
Favola -Moda.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3] bhyve: add a basic driver

[Roman Bogorodskiy]

  Daniel P. Berrange wrote:

 On Thu, Feb 06, 2014 at 12:07:10AM +0400, Roman Bogorodskiy wrote:
Daniel P. Berrange wrote:
  
+virCommandAddArg(cmd, -H); /* vmexit from guest on hlt */
+virCommandAddArg(cmd, -P); /* vmexit from guest on pause */
   
   What's the functional effect of having these set, or not ?
  
  Having that set should make bhyve process terminate on these events (htl
  and pause).
 
 Ok, was just wonder if they needed to be expressed in the
 XML or not, but sounds quite low level so lets ignore it
 for now.
 
  I guess it'd be better not to use it so bhyve process doesn't suddenly
  vanish from libvirt radar?
 
 I don't know really - as long as the driver detects that it has gone
 and marks the guest as shutdown.

I didn't plan to add such detection into an initial driver code,
because it's already rather large and not very easy to review. Is that
fine to leave it as is so far and extend later?

PS BTW, I have updated v4 of this patch which fixes most of the comments
to the current one. I forgot to include checking XML def for apci/apic
(as you have pointed out), but I'm wondering if I should wait for
comments on v4 to submit it along with other fixes or send v5 with
acpi/apic flags right away?

Roman Bogorodskiy

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

Re: [libvirt] [PATCH 3/4] libxl: handle domain shutdown events in a thread

[Michal Privoznik]

On 05.02.2014 18:39, Jim Fehlig wrote:

Handling the domain shutdown event within the event handler seems
a bit unfair to libxl's event machinery.  Domain shutdown could
take considerable time.  E.g. if the shutdown reason is reboot,
the domain must be reaped and then started again.

Spawn a shutdown handler thread to do this work, allowing libxl's
event machinery to go about its business.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
  src/libxl/libxl_driver.c | 132 ---
  1 file changed, 89 insertions(+), 43 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d639011..a1c6c0f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -352,61 +352,107 @@ libxlVmReap(libxlDriverPrivatePtr driver,
  # define VIR_LIBXL_EVENT_CONST const
  #endif

+struct libxlShutdownThreadInfo
+{
+virDomainObjPtr vm;
+libxl_event *event;
+};
+
+
  static void
-libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event)
+libxlDomainShutdownThread(void *opaque)
  {
  libxlDriverPrivatePtr driver = libxl_driver;
-libxlDomainObjPrivatePtr priv = ((virDomainObjPtr)data)-privateData;
-virDomainObjPtr vm = NULL;
+struct libxlShutdownThreadInfo *shutdown_info = opaque;
+virDomainObjPtr vm = shutdown_info-vm;
+libxlDomainObjPrivatePtr priv = vm-privateData;
+libxl_event *ev = shutdown_info-event;
+libxl_ctx *ctx = priv-ctx;
  virObjectEventPtr dom_event = NULL;
-libxl_shutdown_reason xl_reason = event-u.domain_shutdown.shutdown_reason;
-
-if (event-type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) {
-virDomainShutoffReason reason;
-
-/*
- * Similar to the xl implementation, ignore SUSPEND.  Any actions 
needed
- * after calling libxl_domain_suspend() are handled by it's callers.
- */
-if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND)
-goto cleanup;
+libxl_shutdown_reason xl_reason = ev-u.domain_shutdown.shutdown_reason;
+virDomainShutoffReason reason;

-vm = virDomainObjListFindByID(driver-domains, event-domid);
-if (!vm)
-goto cleanup;
+virObjectLock(vm);

-switch (xl_reason) {
-case LIBXL_SHUTDOWN_REASON_POWEROFF:
-case LIBXL_SHUTDOWN_REASON_CRASH:
-if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
-dom_event = virDomainEventLifecycleNewFromObj(vm,
-  VIR_DOMAIN_EVENT_STOPPED,
-  
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
-reason = VIR_DOMAIN_SHUTOFF_CRASHED;
-} else {
-reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
-}
-libxlVmReap(driver, vm, reason);
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
-break;
-case LIBXL_SHUTDOWN_REASON_REBOOT:
-libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
-libxlVmStart(driver, vm, 0, -1);
-break;
-default:
-VIR_INFO(Unhandled shutdown_reason %d, xl_reason);
-break;
-}
+switch (xl_reason) {
+case LIBXL_SHUTDOWN_REASON_POWEROFF:
+case LIBXL_SHUTDOWN_REASON_CRASH:
+if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
+dom_event = virDomainEventLifecycleNewFromObj(vm,
+   VIR_DOMAIN_EVENT_STOPPED,
+   VIR_DOMAIN_EVENT_STOPPED_CRASHED);
+reason = VIR_DOMAIN_SHUTOFF_CRASHED;
+} else {
+reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+}
+libxlVmReap(driver, vm, reason);
+if (!vm-persistent) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
+break;
+case LIBXL_SHUTDOWN_REASON_REBOOT:
+libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+libxlVmStart(driver, vm, 0, -1);
+break;
+default:
+VIR_INFO(Unhandled shutdown_reason %d, xl_reason);
+break;
  }

-cleanup:
  if (vm)
  virObjectUnlock(vm);
  if (dom_event)
  libxlDomainEventQueue(driver, dom_event);
+libxl_event_free(ctx, ev);
+VIR_FREE(shutdown_info);
+}
+
+static void
+libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event)
+{
+virDomainObjPtr vm = data;
+libxlDomainObjPrivatePtr priv = vm-privateData;
+libxl_shutdown_reason xl_reason = event-u.domain_shutdown.shutdown_reason;
+struct libxlShutdownThreadInfo *shutdown_info;
+virThread thread;
+
+if (event-type != LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) {
+VIR_INFO(Unhandled event type %d, 

Re: [libvirt] [PATCH 0/4] libxl: fixes related to concurrency improvements

[Michal Privoznik]

On 05.02.2014 18:39, Jim Fehlig wrote:

While reviving old patches to add job support to the libxl driver,
testing revealed some problems that were difficult to encounter
in the current, more serialized processing approach used in the
driver.

The first patch is a bug fix, plugging leaks of libxlDomainObjPrivate
objects.  The second patch removes the list of libxl timer registrations
maintained in the driver - a hack I was never fond of.  The third patch
moves domain shutdown handling to a thread, instead of doing all the
shutdown work in the event handler.  The fourth patch fixes an issue wrt
child process handling discussed in this thread

http://lists.xen.org/archives/html/xen-devel/2014-01/msg01553.html

Ian Jackson's latest patches on the libxl side are here

http://lists.xen.org/archives/html/xen-devel/2014-02/msg00124.html


Jim Fehlig (4):
   libxl: fix leaking libxlDomainObjPrivate
   libxl: remove list of timer registrations from libxlDomainObjPrivate
   libxl: handle domain shutdown events in a thread
   libxl: improve subprocess handling

  src/libxl/libxl_conf.h   |   5 +-
  src/libxl/libxl_domain.c | 102 ---
  src/libxl/libxl_domain.h |   8 +--
  src/libxl/libxl_driver.c | 135 +++
  4 files changed, 115 insertions(+), 135 deletions(-)



ACK series but see my comment on 3/4 where I'm asking for a pair of 
fixes prior pushing.


Michal

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

Re: [libvirt] [PATCH v3 20/21] lxc: honor link state=up for veth interfaces

[Cedric Bosdonnat]

On Wed, 2014-02-05 at 15:10 +0100, Cédric Bosdonnat wrote:
 direct interfaces are already brought up when creating them.

Just ignore (and drop) this patch, veth interfaces are already up when
starting the container... even thought I couldn't ifdown it ;)

--
Cedric

 ---
  src/lxc/lxc_process.c | 5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
 index ed729f6..6f7ff74 100644
 --- a/src/lxc/lxc_process.c
 +++ b/src/lxc/lxc_process.c
 @@ -245,6 +245,7 @@ char *virLXCProcessSetupInterfaceBridged(virConnectPtr 
 conn,
  char *parentVeth;
  char *containerVeth = NULL;
  virNetDevVPortProfilePtr vport = 
 virDomainNetGetActualVirtPortProfile(net);
 +bool vethUp = false;
  
  VIR_DEBUG(calling vethCreate());
  parentVeth = net-ifname;
 @@ -283,6 +284,10 @@ char *virLXCProcessSetupInterfaceBridged(virConnectPtr 
 conn,
  virDomainConfNWFilterInstantiate(conn, vm-uuid, net)  0)
  goto cleanup;
  
 +vethUp = net-linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
 +if (virNetDevSetOnline(containerVeth, vethUp)  0)
 +goto cleanup;
 +
  ret = containerVeth;
  
  cleanup:


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

Re: [libvirt] [PATCH v3 1/2] BSD: implement nodeGetCPUStats

[Ján Tomko]

On 02/04/2014 10:24 AM, Ján Tomko wrote:
 On 01/28/2014 06:49 PM, Roman Bogorodskiy wrote:
 Implementation obtains CPU usage information using
 kern.cp_time and kern.cp_times sysctl(8)s and reports
 CPU utilization.
 ---
  include/libvirt/libvirt.h.in |   8 
  src/nodeinfo.c   | 104 
 +++
  tools/virsh-host.c   |  11 -
  3 files changed, 121 insertions(+), 2 deletions(-)
 
 ACK
 

Pushed now.

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] virsh: only report filled values in nodecpustats

[Ján Tomko]

On 02/06/2014 01:14 PM, Michal Privoznik wrote:
 On 04.02.2014 10:22, Ján Tomko wrote:
 Rewrite the function to use an array instead of a struct,
 translating the field names to int via an enum.
 ---
   tools/virsh-host.c | 126
 +++--
   1 file changed, 64 insertions(+), 62 deletions(-)
 
 
 ACK, I like this one more.
 

Now pushed. Thank you 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

[libvirt] [PATCH 2/2] qemuxml2argvtest: Set timezone

[Michal Privoznik]

With my recent work on the test, both time() and localtime() are used.
While mocking the former one, we get predictible result for UTC. But
since the latter function uses timezone to get local time, the result of
localtime() is not so predictive. Therefore, we must set the TZ variable
at the beginning of the test. To be able to catch some things that work
just by a blind chance, I'm choosing an exotic timezone that (hopefully)
no libvirt developer resides in.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 .../qemuxml2argv-clock-localtime-basis-localtime.args |  2 +-
 tests/qemuxml2argvtest.c  | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args 
b/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
index 24fe89c..7d39125 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic \
--monitor unix:/tmp/test-monitor,server,nowait -rtc base=2009-02-14T01:31:30 \
+-monitor unix:/tmp/test-monitor,server,nowait -rtc base=2009-02-13T13:31:30 \
 -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none \
 -parallel none
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8b3857..1ee4b59 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -486,6 +486,17 @@ mymain(void)
 if (!abs_top_srcdir)
 abs_top_srcdir = abs_srcdir /..;
 
+/* Set the timezone because we are mocking the time() function.
+ * If we don't do that, then localtime() may return unpredictable
+ * results. In order to detect things that just work by a blind
+ * chance, we need to set an exotic timezone that none libvirt
+ * developer resides in. So if you go for holiday to Niue, don't
+ * send patches but enjoy time off! */
+if (setenv(TZ, Pacific/Niue, 1)  0) {
+perror(setenv);
+return EXIT_FAILURE;
+}
+
 driver.config = virQEMUDriverConfigNew(false);
 VIR_FREE(driver.config-spiceListen);
 VIR_FREE(driver.config-vncListen);
-- 
1.8.5.3

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

[libvirt] [PATCH 1/2] qemuxml2argvmock: Mock time() on non-linux platforms too

[Michal Privoznik]

The qemuxml2argvtest is run on more platforms than linux. For instance
FreeBSD. On these platforms we are, however, not mocking time() which
results in current time being fetched from system and hence tests number
32 and 33 failing.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 tests/qemuxml2argvmock.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index ed9fb13..bff3b0f 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -20,9 +20,8 @@
 
 #include config.h
 
-#ifdef __linux__
-# include internal.h
-# include time.h
+#include internal.h
+#include time.h
 
 time_t time(time_t *t)
 {
@@ -31,7 +30,3 @@ time_t time(time_t *t)
 *t = ret;
 return ret;
 }
-
-#else
-/* Nothing to override on non-__linux__ platforms */
-#endif
-- 
1.8.5.3

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

Re: [libvirt] [PATCH 2/2] qemuxml2argvtest: Set timezone

[Eric Blake]

On 02/06/2014 06:33 AM, Michal Privoznik wrote:
 With my recent work on the test, both time() and localtime() are used.
 While mocking the former one, we get predictible result for UTC. But

s/predictible/predictable/

 since the latter function uses timezone to get local time, the result of
 localtime() is not so predictive. Therefore, we must set the TZ variable
 at the beginning of the test. To be able to catch some things that work
 just by a blind chance, I'm choosing an exotic timezone that (hopefully)
 no libvirt developer resides in.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---

 +/* Set the timezone because we are mocking the time() function.
 + * If we don't do that, then localtime() may return unpredictable
 + * results. In order to detect things that just work by a blind
 + * chance, we need to set an exotic timezone that none libvirt

s/none/no/

 + * developer resides in. So if you go for holiday to Niue, don't
 + * send patches but enjoy time off! */
 +if (setenv(TZ, Pacific/Niue, 1)  0) {

This works on glibc, but is not portable to all platforms.  I'd prefer
we use a POSIX timezone specification:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

 The expanded format (for all TZ s whose value does not have a colon as the 
 first character) is as follows:
 
 stdoffset[dst[offset][,start[/time],end[/time]]]

In fact, you can abuse offset to list a timezone with a half hour offset
from UTC, with no daylight savings, and then guarantee that no developer
uses that timezone:

$ date --utc
Thu Feb  6 13:52:24 UTC 2014
$ TZ=VIR00:30 date
Thu Feb  6 13:22:43 VIR 2014

-- 
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] qemuxml2argvmock: Mock time() on non-linux platforms too

[Roman Bogorodskiy]

  Michal Privoznik wrote:

 The qemuxml2argvtest is run on more platforms than linux. For instance
 FreeBSD. On these platforms we are, however, not mocking time() which
 results in current time being fetched from system and hence tests number
 32 and 33 failing.

This change works on FreeBSD and looks good to me.

Now 'make check' works successfully on FreeBSD.

Roman Bogorodskiy

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

[libvirt] [PATCH v2] qemuxml2argvtest: Set timezone

[Michal Privoznik]

With my recent work on the test, both time() and localtime() are used.
While mocking the former one, we get predictable result for UTC. But
since the latter function uses timezone to get local time, the result of
localtime() is not so predictive. Therefore, we must set the TZ variable
at the beginning of the test. To be able to catch some things that work
just by a blind chance, I'm choosing a virtual timezone that (hopefully)
no libvirt developer resides in.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 .../qemuxml2argv-clock-localtime-basis-localtime.args  |  2 +-
 tests/qemuxml2argvtest.c   | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args 
b/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
index 24fe89c..ab9e979 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime-basis-localtime.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic \
--monitor unix:/tmp/test-monitor,server,nowait -rtc base=2009-02-14T01:31:30 \
+-monitor unix:/tmp/test-monitor,server,nowait -rtc base=2009-02-14T00:01:30 \
 -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none \
 -parallel none
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c8b3857..dae08d5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -486,6 +486,16 @@ mymain(void)
 if (!abs_top_srcdir)
 abs_top_srcdir = abs_srcdir /..;
 
+/* Set the timezone because we are mocking the time() function.
+ * If we don't do that, then localtime() may return unpredictable
+ * results. In order to detect things that just work by a blind
+ * chance, we need to set an virtual timezone that no libvirt
+ * developer resides in. */
+if (setenv(TZ, VIR00:30, 1)  0) {
+perror(setenv);
+return EXIT_FAILURE;
+}
+
 driver.config = virQEMUDriverConfigNew(false);
 VIR_FREE(driver.config-spiceListen);
 VIR_FREE(driver.config-vncListen);
-- 
1.8.5.3

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

[libvirt] Enabling x2apic by default in KvM (was Re: [Qemu-devel] [PATCH] target-i386: enable x2apic by default on more recent CPU models)

[Eduardo Habkost]

On Tue, Feb 04, 2014 at 03:12:43PM +0100, Andreas Färber wrote:
 Am 03.02.2014 20:01, schrieb Eduardo Habkost:
  On Tue, Jan 21, 2014 at 05:13:50PM +0100, Paolo Bonzini wrote:
  Il 21/01/2014 16:51, Andreas Färber ha scritto:
  We already do that for other bits (e.g. XSAVE/OSXSAVE),
  Please point me to the commit, a search for xsave did not come up with a
  commit changing such a thing - either it did not go through my queue or
  it slipped me through: Bugs are no excuse to produce more bugs.
 
  I meant that -cpu SandyBridge with TCG produces a CPU that doesn't
  have XSAVE.
 
  and in fact it
  is the same that we do for KVM: the KVM_GET_SUPPORTED_CPUID result is
  used to trim the generic feature bits.
  Our model definitions are the place to put stuff that real CPUs have.
  Either the CPU has it or it doesn't. If it does, then this patch is
  fully correct and it's TCG's job to mask things out. If we're adding
  artificial flags to the generic model definitions just to make KVM
  faster, then it is wrong - we have a choice of post_initialize and
  realize hooks for that.
 
  It would make TCG faster as well, and there would be no reason
  really to avoid the artificial x2apic on TCG, if TCG implemented
  x2apic at all.
  
  So, the discussion seem to have stalled.
  
  Andreas, are you still against the patch, after the arguments from Paolo
  and me?
 
 Yes, I am. I had proposed to discuss solutions at FOSDEM but Paolo was
 not there, so no solution yet.
 
 My main concern still is that if a CPU does not have a certain feature
 we should not list it as one of its features but add it to its features
 where sensible. Just because TCG filters it out today is not keeping
 anyone from implementing it tomorrow, in which case the emulated CPUs
 would suddenly gain the feature. So my question still is, what rule can
 we apply for enabling x2apic? (something like greater or equal this
 family, etc. - then we can put it in your post_initialize hook so that
 users can still override it)

The rule needs be simple and predictable, so libvirt can know in advance
if the results are going to match what was requested by the user. That's
the problem with enabling it automatically based on some rule more
complex than the existing it's enabled if it's on the CPU model table
or on the command-line rule.

Considering the whole stack, there's a strong reason to enable x2apic by
default on all cases when using KVM. But if keeping the CPU model table
match real CPUs bit-by-bit is more important for QEMU (which I don't
understand why), I believe we will need to ask libvirt to do it (enable
x2apic explicitly in the command-line by default). Or we could make the
CPU models look different in KVM and TCG mode, but I am trying to avoid
that.

But I still believe that enabling x2apic by default in TCG _and_ KVM
mode is easier and harmless. I don't see the value in this academic
exercise of trying to make the CPU model table match real CPUs
bit-by-bit.

I mean: I simply don't expect any user will come to us complaining
because they didn't want x2apic to be enabled by default. And if some
user really cares about that, they can simply use -x2apic in the
command-line. In other words, if it is not going to affect users
negatively, why are we spending energy on it?

-- 
Eduardo

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

Re: [libvirt] [PATCH] Add filesystem support to Qemu's attach_device

[Teto]

These 2 patches should address your points. I've also used
VIR_APPEND_ELEMENT in another function (1st patch).

At your service should you have any other comment.

Matthieu Coudron

2014-02-05 Michal Privoznik mpriv...@redhat.com:
 On 04.02.2014 10:37, Teto wrote:

 Hi,

 The following patch was generated with format patch  checked with
 syntax-check. It is really short and adds a new function
 virDomainFSInsert which is called when Qemu driver is requested t
 attach a filesystem device.

 Matt


 0001-This-commit-allows-to-register-filesystem-in-qemu-vi.patch


  From f8c0612c48c06c61199693743d98c251ba4d887e Mon Sep 17 00:00:00 2001
 From: Mattmatta...@gmail.com
 Date: Mon, 3 Feb 2014 17:42:56 +0100
 Subject: [PATCH] This commit allows to register filesystem in qemu via
 the
   attach_device function (which would previsouly return an error).

 For this purpose I've introduced a new function virDomainFSInsert and
 added the necessary code in the qemu driver.

 It compares filesystems based on their destination folder. So if 2
 filesystems share a same destination, they are considered equal and the
 Qemu driver would reject a new insertion.
 ---
   src/conf/domain_conf.c   | 12 
   src/conf/domain_conf.h   |  1 +
   src/libvirt_private.syms |  1 +
   src/qemu/qemu_driver.c   | 18 ++
   4 files changed, 32 insertions(+)

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 28e24f9..3f4dbfe 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -17933,6 +17933,18 @@ virDiskNameToBusDeviceIndex(virDomainDiskDefPtr
 disk,
   return 0;
   }

 +
 +int
 +virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs)
 +{
 +
 +if (VIR_REALLOC_N(def-fss, def-nfss+1)  0)
 +return -1;
 +
 +def-fss[def-nfss++]  = fs;
 +return 0;
 +}
 +


 While this works perfectly, we can save some lines by calling
 VIR_APPEND_ELEMENT().

   virDomainFSDefPtr
   virDomainGetRootFilesystem(virDomainDefPtr def)
   {
 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index d8f2e49..d749e68 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -2555,6 +2555,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr
 disk,
   int *devIdx);

   virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def);
 +int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs);
   int virDomainFSIndexByName(virDomainDefPtr def, const char *name);
   int virDomainVideoDefaultType(const virDomainDef *def);
   int virDomainVideoDefaultRAM(const virDomainDef *def, int type);
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 1a8d088..e872960 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -221,6 +221,7 @@ virDomainFeatureStateTypeFromString;
   virDomainFeatureStateTypeToString;
   virDomainFSDefFree;
   virDomainFSIndexByName;
 +virDomainFSInsert;
   virDomainFSTypeFromString;
   virDomainFSTypeToString;
   virDomainFSWrpolicyTypeFromString;
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 0128356..f2bac0d 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6606,6 +6606,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr
 qemuCaps,
   virDomainHostdevDefPtr hostdev;
   virDomainLeaseDefPtr lease;
   virDomainControllerDefPtr controller;
 +virDomainFSDefPtr fs;

   switch (dev-type) {
   case VIR_DOMAIN_DEVICE_DISK:
 @@ -6687,6 +6688,23 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr
 qemuCaps,
   dev-data.chr = NULL;
   break;

 +case VIR_DOMAIN_DEVICE_FS:
 +{
 +fs = dev-data.fs;
 +if (virDomainFSIndexByName(vmdef, fs-dst) = 0) {
 +VIR_INFO(Identical FS found);
 +virReportError(VIR_ERR_OPERATION_INVALID,
 + %s, _(Target already exists));
 +return -1;
 +}
 +
 +if (virDomainFSInsert(vmdef, fs)  0) {
 +return -1;
 +}
 +dev-data.fs = NULL;
 +}
 +break;
 +


 There is no need to enclose the body in { }. Nor for VIR_INFO.

 While at this - can you implement the detach counterpart? Again, in config
 level is fine for now.

 However, I believe our push policy doesn't allow in patches that are missing
 real name (first and last one at least). So can you fix that too?

 Michal
From 9235e4874266644af8180512e9920c35cfb0f09a Mon Sep 17 00:00:00 2001
From: Matthieu Coudron matta...@gmail.com
Date: Thu, 6 Feb 2014 15:29:08 +0100
Subject: [PATCH 1/2] Replaced VIR_REALLOC_N by VIR_APPEND_ELEMENT in
 virDomainHostdevInsert so that the code gets shorter and more readable

Signed-off-by: Matthieu Coudron matta...@gmail.com
---
 src/conf/domain_conf.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28e24f9..3f3822e 100644
--- 

Re: [libvirt] [PATCH v2] qemuxml2argvtest: Set timezone

[Ján Tomko]

On 02/06/2014 03:11 PM, Michal Privoznik wrote:
 With my recent work on the test, both time() and localtime() are used.
 While mocking the former one, we get predictable result for UTC. But
 since the latter function uses timezone to get local time, the result of
 localtime() is not so predictive. Therefore, we must set the TZ variable
 at the beginning of the test. To be able to catch some things that work
 just by a blind chance, I'm choosing a virtual timezone that (hopefully)
 no libvirt developer resides in.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  .../qemuxml2argv-clock-localtime-basis-localtime.args  |  2 +-
  tests/qemuxml2argvtest.c   | 10 
 ++
  2 files changed, 11 insertions(+), 1 deletion(-)

ACK, please push.

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 1/2] qemuxml2argvmock: Mock time() on non-linux platforms too

[Ján Tomko]

On 02/06/2014 02:33 PM, Michal Privoznik wrote:
 The qemuxml2argvtest is run on more platforms than linux. For instance
 FreeBSD. On these platforms we are, however, not mocking time() which
 results in current time being fetched from system and hence tests number
 32 and 33 failing.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  tests/qemuxml2argvmock.c | 9 ++---
  1 file changed, 2 insertions(+), 7 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

[libvirt] [PATCH] virpcitest: fix coverity issues

[Pavel Hrdina]

The coverity server complains about the dev-stubDriver that it was
freed by the virPCIDeviceSetStubDriver function, but it somehow don't
get the fact that into the variable is immediately assigned new string.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 tests/virpcitest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 994b300..8ff3b1d 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
 if (!dev)
 goto cleanup;
 
+/* coverity[freed_arg] */
 if (virPCIDeviceSetStubDriver(dev, pci-stub)  0 ||
 virPCIDeviceDetach(dev, NULL, NULL)  0)
 goto cleanup;
@@ -269,6 +270,7 @@ testVirPCIDeviceDetachFail(const void *opaque)
 if (!dev)
 goto cleanup;
 
+/* coverity[freed_arg] */
 if (virPCIDeviceSetStubDriver(dev, vfio-pci)  0)
 goto cleanup;
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] virpcitest: fix coverity issues

[Eric Blake]

On 02/06/2014 08:18 AM, Pavel Hrdina wrote:
 The coverity server complains about the dev-stubDriver that it was
 freed by the virPCIDeviceSetStubDriver function, but it somehow don't
 get the fact that into the variable is immediately assigned new string.
 
 Signed-off-by: Pavel Hrdina phrd...@redhat.com
 ---
  tests/virpcitest.c | 2 ++
  1 file changed, 2 insertions(+)

What's the exact complaint?

 
 diff --git a/tests/virpcitest.c b/tests/virpcitest.c
 index 994b300..8ff3b1d 100644
 --- a/tests/virpcitest.c
 +++ b/tests/virpcitest.c
 @@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
  if (!dev)
  goto cleanup;
  
 +/* coverity[freed_arg] */
  if (virPCIDeviceSetStubDriver(dev, pci-stub)  0 ||
  virPCIDeviceDetach(dev, NULL, NULL)  0)
  goto cleanup;

Is this something where an sa_assert() could do the trick in a more
tool-agnostic manner?

-- 
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] CPU models and feature probing (was Re: [Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU) features into properties

[Eduardo Habkost]

(CCing libvir-list again, as this is continuing a discussion about a
subject that interests libvirt developers, from another thread.)

On Thu, Feb 06, 2014 at 04:51:17PM +0100, Andreas Färber wrote:
 Am 06.02.2014 16:19, schrieb Igor Mammedov:
  On Wed, 5 Feb 2014 17:52:16 +0100
  Igor Mammedov imamm...@redhat.com wrote:
  
  On Wed, 05 Feb 2014 17:14:27 +0100
  Andreas Färber afaer...@suse.de wrote:
 
  Am 05.02.2014 15:40, schrieb Igor Mammedov:
  On Sun, 15 Dec 2013 23:50:47 +0100
  Andreas Färber afaer...@suse.de wrote:
 
  Am 27.11.2013 23:28, schrieb Igor Mammedov:
  Igor Mammedov (16):
target-i386: cleanup 'foo' feature handling'
target-i386: cleanup 'foo=val' feature handling
 
  Thanks, I've queued these on qom-cpu-next:
  https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
 
target-i386: cpu: convert 'level' to static property
target-i386: cpu: convert 'xlevel' to static property
target-i386: cpu: convert 'family' to static property
target-i386: cpu: convert 'model' to static property
target-i386: cpu: convert 'stepping' to static property
target-i386: cpu: convert 'vendor' to static property
target-i386: cpu: convert 'model-id' to static property
target-i386: cpu: convert 'tsc-frequency' to static property
 
  But I still don't see the utility of this conversion after all the
  discussions we've had... :(
  It seems there is movement to make DEVICE self describing for purpose
  of QAPI schema introspection, where static properties would be used
  (dynamic ones are not suitable for this purpose)
 
  Do you have a pointer to such a discussion? Sounds like I was not
  involved and Anthony probably not either...
  Not at the moment, CCing people who might know more about the topic.
 
  But just thinking about creating QAPI schema for devices, it's not really
  possible to generate one using dynamic properties (unless one resorts to
  creating every supported device instance), while arrays of static 
  properties
  are there for every device and simplistically speaking just need conversion
  to schema format.
  
  There is one more reason to use static properties for external user-settable
  properties when it comes to device, i.e. to get list of properties user 
  could
  use command:
  #qemu -device x86_64-cpu,?
  x86_64-cpu.pmu=boolean
  x86_64-cpu.hv-spinlocks=int
  x86_64-cpu.hv-relaxed=boolean
  x86_64-cpu.hv-vapic=boolean
  x86_64-cpu.check=boolean
  x86_64-cpu.enforce=boolean
  
  which now yields only partial properties user is interested in, above
  mentioned conversion patches make previously not available properties
  visible to user via typical interface, perhaps eventually we could
  drop list_cpu() interface in favor of -device foo-cpu,? command.
 
 We already brought that up specifically for decision on a KVM call and
 Anthony's clear statement was that the expected way for management tools
 to discover properties was to instantiate the object and run qom-list on it.

As far as I remember, this was decided as the recommended way to list
the feature _default values_ (e.g. discover which features are going to
be enabled on each CPU model). Is this really the recommended way to
discover which properties can be set on device_add, too? We are not
going to have any other instrospection mechanism that won't require
objects to be created?

 
 It is a known issue, both for info qtree and -device, that they do not
 list all properties. But I don't want to repeat this discussion over and
 over again: Paolo's patches for static properties were rejected by
 Anthony, therefore I could not queue them on qom-next back then and
 therefore I had to code my properties for the X86CPU (which was not yet
 a device back then) the new QOM way, and now you're trying to override
 Anthony's decision in forcing me to accept patches that Anthony had
 vetoed against!
 
 If you or libvirt need all properties for -device, then send a patch. No
 one did for two years, so apparently no one cared.
 
 Static properties are considered a valid, convenient way to define
 properties for a device but not the sole one for a device or object.
 Using info qtree or -device as justification for implementation
 decisions is backwards and wrong since those are considered legacy.

So, let me ask again, explicitly: we are not going to ever have a
QMP-based interface that will allow all available device_add options to
be queried without instantiating an object first? This really surprises
me.

(That's not a question just for Andreas. I would like to hear from
Paolo, Anthony, and others.)


 
 And specifically for libvirt Eduardo pushed into a release properties
 that could be used to inspect CPUIDs. If that's not being used by
 libvirt, as Eduardo seems to imply now, why did we put work in that in
 the first place?

It is not being used by libvirt because the current interface is
unusable without running QEMU once for each CPU model. With the CPU
model subclasses, we will be able to make 

Re: [libvirt] [PATCH] Add filesystem support to Qemu's attach_device

[Michal Privoznik]

On 06.02.2014 15:51, Teto wrote:

These 2 patches should address your points. I've also used
VIR_APPEND_ELEMENT in another function (1st patch).

At your service should you have any other comment.

Matthieu Coudron


snip/

0001-Replaced-VIR_REALLOC_N-by-VIR_APPEND_ELEMENT-in-virD.patch


 From 9235e4874266644af8180512e9920c35cfb0f09a Mon Sep 17 00:00:00 2001
From: Matthieu Coudronmatta...@gmail.com
Date: Thu, 6 Feb 2014 15:29:08 +0100
Subject: [PATCH 1/2] Replaced VIR_REALLOC_N by VIR_APPEND_ELEMENT in
  virDomainHostdevInsert so that the code gets shorter and more readable

Signed-off-by: Matthieu Coudronmatta...@gmail.com
---
  src/conf/domain_conf.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28e24f9..3f3822e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9868,10 +9868,8 @@ virDomainChrTargetTypeToString(int deviceType,
  int
  virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr hostdev)
  {
-if (VIR_REALLOC_N(def-hostdevs, def-nhostdevs + 1)  0)
-return -1;
-def-hostdevs[def-nhostdevs++]  = hostdev;
-return 0;
+
+return VIR_APPEND_ELEMENT(def-hostdevs, def-nhostdevs, hostdev);
  }



virDomainHostdevRemove could be changed as well :) I've taken a chance 
and did it.



  virDomainHostdevDefPtr
-- 1.8.3.2


0002-filesystem-support-in-device-addition-removal-for-th.patch


 From ed71d16b697c5a17946e5bef9fb74e6ac5a52fb0 Mon Sep 17 00:00:00 2001
From: Matthieu Coudronmatta...@gmail.com
Date: Thu, 6 Feb 2014 15:30:07 +0100
Subject: [PATCH 2/2] filesystem support in device addition/removal for the
  Qemu driver

This commit allows to attach/detach a filesystem device in qemu (which would 
previously return an error).

For this purpose I've introduced 2 new functions virDomainFSInsert and 
virDomainFSRemove and
added the necessary code in the qemu driver.

It compares filesystems based on their destination folder. So if 2
filesystems share a same destination, they are considered equal and the
Qemu driver would reject a new insertion.

Signed-off-by: Matthieu Coudronmatta...@gmail.com
---
  src/conf/domain_conf.c   | 30 ++
  src/conf/domain_conf.h   |  3 +++
  src/libvirt_private.syms |  2 ++
  src/qemu/qemu_driver.c   | 30 ++
  4 files changed, 65 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3f3822e..9e75b21 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17931,6 +17931,36 @@ virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
  return 0;
  }

+
+int
+virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs)
+{
+
+return VIR_APPEND_ELEMENT(def-fss, def-nfss, fs);
+}
+
+virDomainFSDefPtr
+virDomainFSRemove(virDomainDefPtr def, size_t i)
+{
+virDomainFSDefPtr fs = def-fss[i];
+
+if (def-nfss  1) {
+
+memmove(def-fss + i,
+def-fss + i + 1,
+sizeof(*def-fss) *
+(def-nfss - (i + 1)));
+def-nfss--;
+if (VIR_REALLOC_N(def-fss, def-nfss)  0) {
+/* ignore, harmless */
+}
+} else {
+VIR_FREE(def-fss);
+def-nfss = 0;
+}
+return fs;
+}
+


Again, we have VIR_DELETE_ELEMENT, but I've changed this too.


  virDomainFSDefPtr
  virDomainGetRootFilesystem(virDomainDefPtr def)
  {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d8f2e49..9acb105 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2555,7 +2555,10 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
  int *devIdx);

  virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def);
+int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs);
  int virDomainFSIndexByName(virDomainDefPtr def, const char *name);
+virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i);
+
  int virDomainVideoDefaultType(const virDomainDef *def);
  int virDomainVideoDefaultRAM(const virDomainDef *def, int type);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5a7637..2c9536a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -221,6 +221,8 @@ virDomainFeatureStateTypeFromString;
  virDomainFeatureStateTypeToString;
  virDomainFSDefFree;
  virDomainFSIndexByName;
+virDomainFSInsert;
+virDomainFSRemove;
  virDomainFSTypeFromString;
  virDomainFSTypeToString;
  virDomainFSWrpolicyTypeFromString;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 38a48db..8d7b228 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6606,6 +6606,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
  virDomainHostdevDefPtr hostdev;
  virDomainLeaseDefPtr lease;
  virDomainControllerDefPtr controller;
+virDomainFSDefPtr fs;

  switch (dev-type) {
  case VIR_DOMAIN_DEVICE_DISK:
@@ -6687,6 +6688,20 @@ 

[libvirt] [PATCH] qemu: keep pre-migration domain state after failed migration

[Martin Kletzander]

Couple of codepaths shared the same code which can be moved out to a
function and on one of such places, qemuMigrationConfirmPhase(), the
domain was resumed even if it wasn't running before the migration
started.

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

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_domain.h|   3 +-
 src/qemu/qemu_migration.c | 117 +-
 2 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6a92351..84624f9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1,7 +1,7 @@
 /*
  * qemu_domain.h: QEMU domain private state
  *
- * 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
@@ -161,6 +161,7 @@ struct _qemuDomainObjPrivate {
 char *origname;
 int nbdPort; /* Port used for migration with NBD */
 unsigned short migrationPort;
+int preMigrationState;

 virChrdevsPtr devs;

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 407fb70..1a29efa 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1075,6 +1075,53 @@ error:
 return NULL;
 }

+static void
+qemuDomainObjStorePreMigrationState(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+priv-preMigrationState = virDomainObjGetState(vm, NULL);
+
+VIR_DEBUG(Storing pre-migration state=%d domain=%p,
+  priv-preMigrationState, vm);
+}
+
+/* Returns true if the domain was resumed, false otherwise */
+static bool
+qemuDomainObjRestorePreMigrationState(virConnectPtr conn, virDomainObjPtr vm)
+{
+virQEMUDriverPtr driver = conn-privateData;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int state = virDomainObjGetState(vm, NULL);
+bool ret = false;
+
+VIR_DEBUG(driver=%p, vm=%p, pre-mig-state=%d, state=%d,
+  driver, vm, priv-preMigrationState, state);
+
+if (state == VIR_DOMAIN_PAUSED 
+priv-preMigrationState == VIR_DOMAIN_RUNNING) {
+/* This is basically the only restore possibility that's safe
+ * and we should attemt to do */
+
+VIR_DEBUG(Restoring pre-migration state due to migration error);
+
+/* we got here through some sort of failure; start the domain again */
+if (qemuProcessStartCPUs(driver, vm, conn,
+ VIR_DOMAIN_RUNNING_MIGRATION_CANCELED,
+ QEMU_ASYNC_JOB_MIGRATION_OUT)  0) {
+/* Hm, we already know we are in error here.  We don't want to
+ * overwrite the previous error, though, so we just throw something
+ * to the logs and hope for the best */
+VIR_ERROR(_(Failed to resume guest %s after failure), 
vm-def-name);
+goto cleanup;
+}
+ret = true;
+}
+
+ cleanup:
+priv-preMigrationState = VIR_DOMAIN_NOSTATE;
+return ret;
+}
+
 /**
  * qemuMigrationStartNBDServer:
  * @driver: qemu driver
@@ -1952,8 +1999,8 @@ cleanup:


 /* The caller is supposed to lock the vm and start a migration job. */
-static char
-*qemuMigrationBeginPhase(virQEMUDriverPtr driver,
+static char *
+qemuMigrationBeginPhase(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  const char *xmlin,
  const char *dname,
@@ -2075,6 +2122,8 @@ qemuMigrationBegin(virConnectPtr conn,
 asyncJob = QEMU_ASYNC_JOB_NONE;
 }

+qemuDomainObjStorePreMigrationState(vm);
+
 if (!virDomainObjIsActive(vm)  !(flags  VIR_MIGRATE_OFFLINE)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(domain is not running));
@@ -2744,22 +2793,12 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
 /* cancel any outstanding NBD jobs */
 qemuMigrationCancelDriveMirror(mig, driver, vm);

-/* run 'cont' on the destination, which allows migration on qemu
- * = 0.10.6 to work properly.  This isn't strictly necessary on
- * older qemu's, but it also doesn't hurt anything there
- */
-if (qemuProcessStartCPUs(driver, vm, conn,
- VIR_DOMAIN_RUNNING_MIGRATED,
- QEMU_ASYNC_JOB_MIGRATION_OUT)  0) {
-if (virGetLastError() == NULL)
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   %s, _(resume operation failed));
-goto cleanup;
+if (qemuDomainObjRestorePreMigrationState(conn, vm)) {
+event = virDomainEventLifecycleNewFromObj(vm,
+  VIR_DOMAIN_EVENT_RESUMED,
+  
VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
 }

-event = 

Re: [libvirt] [PATCH] virpcitest: fix coverity issues

[Pavel Hrdina]

On 6.2.2014 16:48, Eric Blake wrote:

On 02/06/2014 08:18 AM, Pavel Hrdina wrote:

The coverity server complains about the dev-stubDriver that it was
freed by the virPCIDeviceSetStubDriver function, but it somehow don't
get the fact that into the variable is immediately assigned new string.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
  tests/virpcitest.c | 2 ++
  1 file changed, 2 insertions(+)


What's the exact complaint?


The first one that coverity don't get that after free the new string is
assigned into the dev-stubDriver:


Event freed_arg: virPCIDeviceSetStubDriver frees dev-stubDriver.


This is the root cause for another complaint that the dev-stubDriver is
dereferenced after free:


Event deref_arg: Calling virPCIDeviceDetach dereferences freed
pointer dev-stubDriver.






diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 994b300..8ff3b1d 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
  if (!dev)
  goto cleanup;

+/* coverity[freed_arg] */
  if (virPCIDeviceSetStubDriver(dev, pci-stub)  0 ||
  virPCIDeviceDetach(dev, NULL, NULL)  0)
  goto cleanup;


Is this something where an sa_assert() could do the trick in a more
tool-agnostic manner?



I'm looking at the code again and probably the issue found by coverity
could happen if the STRDUP fails inside of the
virPCIDeviceSetStubDriver.


This is a corner case where it could leads to creating a wrong modprobe
command because the dev-stubDriver also as driver and then as module
wouldn't be included in the command line.


virPCIDeviceDetach()-virPCIProbeStubDriver()-virKModLoad()-
doModprobe()


For the virKmodLoad() function the coverity found this issue:


Event deref_parm_in_call:   Function virKModLoad dereferences
driver. (The dereference is assumed on the basis of the 'nonnull'
parameter attribute.)


I'm not sure what to do with this. At first I've thought that it's only
the coverity mistake.

Thanks for the review,

Pavel

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

[libvirt] [PATCHv3 1/2] Split out bind() from virPortAllocatorAcquire

[Ján Tomko]

---
 src/util/virportallocator.c | 72 -
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 694f191..5d51434 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -99,19 +99,59 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
 return pa;
 }
 
+static int virPortAllocatorBindToPort(bool *used,
+  unsigned short port)
+{
+struct sockaddr_in addr = {
+.sin_family = AF_INET,
+.sin_port = htons(port),
+.sin_addr.s_addr = htonl(INADDR_ANY)
+};
+int reuse = 1;
+int ret = -1;
+int fd = -1;
+
+*used = false;
+
+fd = socket(PF_INET, SOCK_STREAM, 0);
+if (fd  0) {
+virReportSystemError(errno, %s, _(Unable to open test socket));
+goto cleanup;
+}
+
+if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)reuse,
+   sizeof(reuse))  0) {
+virReportSystemError(errno, %s,
+ _(Unable to set socket reuse addr flag));
+goto cleanup;
+}
+
+if (bind(fd, (struct sockaddr*)addr, sizeof(addr))  0) {
+if (errno == EADDRINUSE) {
+*used = true;
+ret = 0;
+} else {
+virReportSystemError(errno, _(Unable to bind to port %d), port);
+}
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FORCE_CLOSE(fd);
+return ret;
+}
+
 int virPortAllocatorAcquire(virPortAllocatorPtr pa,
 unsigned short *port)
 {
 int ret = -1;
 size_t i;
-int fd = -1;
 
 *port = 0;
 virObjectLock(pa);
 
 for (i = pa-start; i = pa-end  !*port; i++) {
-int reuse = 1;
-struct sockaddr_in addr;
 bool used = false;
 
 if (virBitmapGetBit(pa-bitmap,
@@ -124,31 +164,10 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
 if (used)
 continue;
 
-addr.sin_family = AF_INET;
-addr.sin_port = htons(i);
-addr.sin_addr.s_addr = htonl(INADDR_ANY);
-fd = socket(PF_INET, SOCK_STREAM, 0);
-if (fd  0) {
-virReportSystemError(errno, %s,
- _(Unable to open test socket));
+if (virPortAllocatorBindToPort(used, i)  0)
 goto cleanup;
-}
-
-if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)reuse, 
sizeof(reuse))  0) {
-virReportSystemError(errno, %s,
- _(Unable to set socket reuse addr flag));
-goto cleanup;
-}
 
-if (bind(fd, (struct sockaddr*)addr, sizeof(addr))  0) {
-if (errno != EADDRINUSE) {
-virReportSystemError(errno,
- _(Unable to bind to port %zu), i);
-goto cleanup;
-}
-/* In use, try next */
-VIR_FORCE_CLOSE(fd);
-} else {
+if (!used) {
 /* Add port to bitmap of reserved ports */
 if (virBitmapSetBit(pa-bitmap,
 i - pa-start)  0) {
@@ -168,7 +187,6 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
 }
 cleanup:
 virObjectUnlock(pa);
-VIR_FORCE_CLOSE(fd);
 return ret;
 }
 
-- 
1.8.3.2

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

[libvirt] [PATCHv3 0/2] Support IPv6 in port allocator

[Ján Tomko]

v1: Support IPv6 in port allocator
https://www.redhat.com/archives/libvir-list/2013-October/msg7.html

v2: https://www.redhat.com/archives/libvir-list/2013-October/msg01313.html
  bind to v4 and v6 separately

v3:
  fix the embarrasing bug of hardcoding AF_INET anyway
  added a test that mocks a v4-only system even on systems with IPv6
compiled in

Ján Tomko (2):
  Split out bind() from virPortAllocatorAcquire
  Support IPv6 in port allocator

 src/util/virportallocator.c  | 106 +++
 tests/virportallocatortest.c |  68 +--
 2 files changed, 143 insertions(+), 31 deletions(-)

-- 
1.8.3.2

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

[libvirt] [PATCHv3 2/2] Support IPv6 in port allocator

[Ján Tomko]

Also try to bind on IPv6 to check if the port is occupied.

Change the mocked bind in the test to return EADDRINUSE
for some ports only for the IPv4/IPv6 socket if we're testing
on a host with IPv6 compiled in.

Also mock socket() to make it fail with EAFNOTSUPPORTED
if LIBVIRT_TEST_IPV4ONLY is set in the environment, to
simulate a host without IPv6 support in the kernel. The
tests are repeated again with this variable set.

https://bugzilla.redhat.com/show_bug.cgi?id=1025407
---
 src/util/virportallocator.c  | 46 +-
 tests/virportallocatortest.c | 68 ++--
 2 files changed, 104 insertions(+), 10 deletions(-)

diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 5d51434..06174b0 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -100,21 +100,45 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
 }
 
 static int virPortAllocatorBindToPort(bool *used,
-  unsigned short port)
+  unsigned short port,
+  int family)
 {
-struct sockaddr_in addr = {
+struct sockaddr_in6 addr6 = {
+.sin6_family = AF_INET6,
+.sin6_port = htons(port),
+.sin6_addr = IN6ADDR_ANY_INIT,
+};
+struct sockaddr_in addr4 = {
 .sin_family = AF_INET,
 .sin_port = htons(port),
 .sin_addr.s_addr = htonl(INADDR_ANY)
 };
+struct sockaddr* addr;
+size_t addrlen;
+int v6only = 1;
 int reuse = 1;
 int ret = -1;
 int fd = -1;
+bool ipv6 = false;
+
+if (family == AF_INET6) {
+addr = (struct sockaddr*)addr6;
+addrlen = sizeof(addr6);
+ipv6 = true;
+} else if (family == AF_INET) {
+addr = (struct sockaddr*)addr4;
+addrlen = sizeof(addr4);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown family %d), family);
+return -1;
+}
 
 *used = false;
 
-fd = socket(PF_INET, SOCK_STREAM, 0);
+fd = socket(family, SOCK_STREAM, 0);
 if (fd  0) {
+if (errno == EAFNOSUPPORT)
+return 0;
 virReportSystemError(errno, %s, _(Unable to open test socket));
 goto cleanup;
 }
@@ -126,7 +150,14 @@ static int virPortAllocatorBindToPort(bool *used,
 goto cleanup;
 }
 
-if (bind(fd, (struct sockaddr*)addr, sizeof(addr))  0) {
+if (ipv6  setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)v6only,
+   sizeof(v6only))  0) {
+virReportSystemError(errno, %s,
+ _(Unable to set IPV6_V6ONLY flag));
+goto cleanup;
+}
+
+if (bind(fd, addr, addrlen)  0) {
 if (errno == EADDRINUSE) {
 *used = true;
 ret = 0;
@@ -152,7 +183,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
 virObjectLock(pa);
 
 for (i = pa-start; i = pa-end  !*port; i++) {
-bool used = false;
+bool used = false, v6used = false;
 
 if (virBitmapGetBit(pa-bitmap,
 i - pa-start, used)  0) {
@@ -164,10 +195,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
 if (used)
 continue;
 
-if (virPortAllocatorBindToPort(used, i)  0)
+if (virPortAllocatorBindToPort(v6used, i, AF_INET6)  0 ||
+virPortAllocatorBindToPort(used, i, AF_INET)  0)
 goto cleanup;
 
-if (!used) {
+if (!used  !v6used) {
 /* Add port to bitmap of reserved ports */
 if (virBitmapSetBit(pa-bitmap,
 i - pa-start)  0) {
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 721356e..7fe18df 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Red Hat, Inc.
+ * Copyright (C) 2013-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
@@ -19,6 +19,8 @@
  */
 
 #include config.h
+#include stdlib.h
+#include virfile.h
 
 #ifdef MOCK_HELPER
 # include internal.h
@@ -26,6 +28,47 @@
 # include errno.h
 # include arpa/inet.h
 # include netinet/in.h
+# include stdio.h
+# include dlfcn.h
+
+static bool host_has_ipv6 = false;
+static int (*realsocket)(int domain, int type, int protocol);
+
+static void init_syms(void)
+{
+int fd;
+
+if (realsocket)
+return;
+
+realsocket = dlsym(RTLD_NEXT, socket);
+
+if (!realsocket) {
+fprintf(stderr, Unable to find 'socket' symbol\n);
+abort();
+}
+
+fd = realsocket(AF_INET6, SOCK_STREAM, 0);
+if (fd  0)
+return;
+
+host_has_ipv6 = true;
+VIR_FORCE_CLOSE(fd);
+}
+
+int socket(int domain,
+   int type,
+   int protocol)
+{
+init_syms();
+
+if (getenv(LIBVIRT_TEST_IPV4ONLY)  domain == 

[libvirt] [PATCH] add option to enforce minimal pagesize for hugetlbfs backed guests

[Marcelo Tosatti]

Require a minimal pagesize for hugetlbfs backed guests. Fail guest 
initialization
if hugetlbfs mount is configured with smaller page size.

Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fd02864..e28d182 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -632,6 +632,10 @@
   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
+  dtcodepagesize/code/dt
+  ddThis tells the hypervisor that the guest should refuse to start
+  in case of failure to allocate guest memory with hugepages equal
+  to or larger than the specified size/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/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28e24f9..babb745 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11274,6 +11274,10 @@ virDomainDefParseXML(xmlDocPtr xml,
  def-mem.swap_hard_limit, false)  0)
 goto error;
 
+if (virDomainParseMemory(./memoryBacking/hugepages/pagesize[1], ctxt,
+ def-mem.page_size, false)  0)
+goto error;
+
 n = virXPathULong(string(./vcpu[1]), ctxt, count);
 if (n == -2) {
 virReportError(VIR_ERR_XML_ERROR, %s,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d8f2e49..03a900d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1984,6 +1984,7 @@ struct _virDomainDef {
 unsigned long long soft_limit; /* in kibibytes */
 unsigned long long min_guarantee; /* in kibibytes */
 unsigned long long swap_hard_limit; /* in kibibytes */
+unsigned long long page_size; /* in kibibytes */
 } mem;
 unsigned short vcpus;
 unsigned short maxvcpus;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8bcd98e..cd5e1c8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3570,6 +3570,33 @@ cleanup:
 return ret;
 }
 
+#ifdef __linux__
+
+#include sys/vfs.h
+
+#define HUGETLBFS_MAGIC   0x958458f6
+
+static long gethugepagesize(const char *path)
+{
+struct statfs fs;
+int ret;
+
+do {
+ret = statfs(path, fs);
+} while (ret != 0  errno == EINTR);
+
+if (ret != 0) {
+perror(path);
+return 0;
+}
+
+if (fs.f_type != HUGETLBFS_MAGIC)
+return 0;
+
+return fs.f_bsize;
+}
+#endif
+
 
 int qemuProcessStart(virConnectPtr conn,
  virQEMUDriverPtr driver,
@@ -3712,6 +3739,31 @@ int qemuProcessStart(virConnectPtr conn,
 %s, _(Unable to set huge path in security driver));
 goto cleanup;
 }
+
+if (vm-def-mem.page_size) {
+#ifdef __linux__
+unsigned long hpagesize = gethugepagesize(cfg-hugepagePath);
+
+if (!hpagesize) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(Unable to stat hugepage path));
+goto cleanup;
+}
+
+hpagesize /= 1024;
+
+if (hpagesize  vm-def-mem.page_size) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+_(Error: hugetlbfs page size=%ld  pagesize=%lld),
+hpagesize, vm-def-mem.page_size);
+goto cleanup;
+}
+#else
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(pagesize option unsupported));
+goto cleanup;
+#endif
+}
 }
 
 /* Ensure no historical cgroup for this VM is lying around bogus

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

Re: [libvirt] CPU models and feature probing (was Re: [Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU) features into properties

[Andreas Färber]

Am 06.02.2014 17:16, schrieb Eduardo Habkost:
 (CCing libvir-list again, as this is continuing a discussion about a
 subject that interests libvirt developers, from another thread.)
 
 On Thu, Feb 06, 2014 at 04:51:17PM +0100, Andreas Färber wrote:
 Am 06.02.2014 16:19, schrieb Igor Mammedov:
 On Wed, 5 Feb 2014 17:52:16 +0100
 Igor Mammedov imamm...@redhat.com wrote:

 On Wed, 05 Feb 2014 17:14:27 +0100
 Andreas Färber afaer...@suse.de wrote:

 Am 05.02.2014 15:40, schrieb Igor Mammedov:
 On Sun, 15 Dec 2013 23:50:47 +0100
 Andreas Färber afaer...@suse.de wrote:

 Am 27.11.2013 23:28, schrieb Igor Mammedov:
 Igor Mammedov (16):
   target-i386: cleanup 'foo' feature handling'
   target-i386: cleanup 'foo=val' feature handling

 Thanks, I've queued these on qom-cpu-next:
 https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next

   target-i386: cpu: convert 'level' to static property
   target-i386: cpu: convert 'xlevel' to static property
   target-i386: cpu: convert 'family' to static property
   target-i386: cpu: convert 'model' to static property
   target-i386: cpu: convert 'stepping' to static property
   target-i386: cpu: convert 'vendor' to static property
   target-i386: cpu: convert 'model-id' to static property
   target-i386: cpu: convert 'tsc-frequency' to static property

 But I still don't see the utility of this conversion after all the
 discussions we've had... :(
 It seems there is movement to make DEVICE self describing for purpose
 of QAPI schema introspection, where static properties would be used
 (dynamic ones are not suitable for this purpose)

 Do you have a pointer to such a discussion? Sounds like I was not
 involved and Anthony probably not either...
 Not at the moment, CCing people who might know more about the topic.

 But just thinking about creating QAPI schema for devices, it's not really
 possible to generate one using dynamic properties (unless one resorts to
 creating every supported device instance), while arrays of static 
 properties
 are there for every device and simplistically speaking just need conversion
 to schema format.

 There is one more reason to use static properties for external user-settable
 properties when it comes to device, i.e. to get list of properties user 
 could
 use command:
 #qemu -device x86_64-cpu,?
 x86_64-cpu.pmu=boolean
 x86_64-cpu.hv-spinlocks=int
 x86_64-cpu.hv-relaxed=boolean
 x86_64-cpu.hv-vapic=boolean
 x86_64-cpu.check=boolean
 x86_64-cpu.enforce=boolean

 which now yields only partial properties user is interested in, above
 mentioned conversion patches make previously not available properties
 visible to user via typical interface, perhaps eventually we could
 drop list_cpu() interface in favor of -device foo-cpu,? command.

 We already brought that up specifically for decision on a KVM call and
 Anthony's clear statement was that the expected way for management tools
 to discover properties was to instantiate the object and run qom-list on it.
 
 As far as I remember, this was decided as the recommended way to list
 the feature _default values_ (e.g. discover which features are going to
 be enabled on each CPU model). Is this really the recommended way to
 discover which properties can be set on device_add, too? We are not
 going to have any other instrospection mechanism that won't require
 objects to be created?

It is, and no, no other introspection mechanism on the horizon.

I specifically suggested to have a copy of the ObjectProperty list in
ObjectClass (today it is just in Object) but Anthony rejected that idea
due to the dynamic nature of properties in QOM and pointed us to said
qom-list and object instantiation as the sole means for discovering
availability of properties.

And it's true that we could in fact just instantiate the object for
-device foo,? - it's just that nobody wrote code for that. I didn't do
the original QOM conversion so I don't feel guilty, I don't normally use
-device foo,? so not affected, I followed Anthony's instructions for how
to and how not to implement things. In a few cases Anthony has changed
his mind (method inheritence for instance) but for that you'll need to
discuss with him and not just argue with me about something that Anthony
has decided in the past. That's just pissing me off because it feels
like a waste of my time.

 It is a known issue, both for info qtree and -device, that they do not
 list all properties. But I don't want to repeat this discussion over and
 over again: Paolo's patches for static properties were rejected by
 Anthony, therefore I could not queue them on qom-next back then and
 therefore I had to code my properties for the X86CPU (which was not yet
 a device back then) the new QOM way, and now you're trying to override
 Anthony's decision in forcing me to accept patches that Anthony had
 vetoed against!

 If you or libvirt need all properties for -device, then send a patch. No
 one did for two years, so apparently no one cared.

 Static properties are considered 

Re: [libvirt] [PATCH] qemu: keep pre-migration domain state after failed migration

[Jiri Denemark]

On Thu, Feb 06, 2014 at 17:33:14 +0100, Martin Kletzander wrote:
 Couple of codepaths shared the same code which can be moved out to a
 function and on one of such places, qemuMigrationConfirmPhase(), the
 domain was resumed even if it wasn't running before the migration
 started.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1057407
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/qemu/qemu_domain.h|   3 +-
  src/qemu/qemu_migration.c | 117 
 +-
  2 files changed, 66 insertions(+), 54 deletions(-)
 
 diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
 index 6a92351..84624f9 100644
 --- a/src/qemu/qemu_domain.h
 +++ b/src/qemu/qemu_domain.h
 @@ -1,7 +1,7 @@
  /*
   * qemu_domain.h: QEMU domain private state
   *
 - * 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
 @@ -161,6 +161,7 @@ struct _qemuDomainObjPrivate {
  char *origname;
  int nbdPort; /* Port used for migration with NBD */
  unsigned short migrationPort;
 +int preMigrationState;
 
  virChrdevsPtr devs;
 
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 407fb70..1a29efa 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -1075,6 +1075,53 @@ error:
  return NULL;
  }
 
 +static void
 +qemuDomainObjStorePreMigrationState(virDomainObjPtr vm)
 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +priv-preMigrationState = virDomainObjGetState(vm, NULL);
 +
 +VIR_DEBUG(Storing pre-migration state=%d domain=%p,
 +  priv-preMigrationState, vm);
 +}

We are in qemu_migration.c file so this function and the one below
should use qemuMigration prefix.

 +
 +/* Returns true if the domain was resumed, false otherwise */
 +static bool
 +qemuDomainObjRestorePreMigrationState(virConnectPtr conn, virDomainObjPtr vm)
 +{
 +virQEMUDriverPtr driver = conn-privateData;
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +int state = virDomainObjGetState(vm, NULL);
 +bool ret = false;
 +
 +VIR_DEBUG(driver=%p, vm=%p, pre-mig-state=%d, state=%d,
 +  driver, vm, priv-preMigrationState, state);
 +
 +if (state == VIR_DOMAIN_PAUSED 
 +priv-preMigrationState == VIR_DOMAIN_RUNNING) {
 +/* This is basically the only restore possibility that's safe
 + * and we should attemt to do */
 +
 +VIR_DEBUG(Restoring pre-migration state due to migration error);
 +
 +/* we got here through some sort of failure; start the domain again 
 */
 +if (qemuProcessStartCPUs(driver, vm, conn,
 + VIR_DOMAIN_RUNNING_MIGRATION_CANCELED,
 + QEMU_ASYNC_JOB_MIGRATION_OUT)  0) {
 +/* Hm, we already know we are in error here.  We don't want to
 + * overwrite the previous error, though, so we just throw 
 something
 + * to the logs and hope for the best */
 +VIR_ERROR(_(Failed to resume guest %s after failure), 
 vm-def-name);
 +goto cleanup;
 +}
 +ret = true;
 +}
 +
 + cleanup:
 +priv-preMigrationState = VIR_DOMAIN_NOSTATE;
 +return ret;
 +}
 +
  /**
   * qemuMigrationStartNBDServer:
   * @driver: qemu driver
 @@ -1952,8 +1999,8 @@ cleanup:
 
 
  /* The caller is supposed to lock the vm and start a migration job. */
 -static char
 -*qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 +static char *
 +qemuMigrationBeginPhase(virQEMUDriverPtr driver,

Heh, it took me a while to see what you did :-) However, the following
lines are not indented correctly now...

   virDomainObjPtr vm,
   const char *xmlin,
   const char *dname,
 @@ -2075,6 +2122,8 @@ qemuMigrationBegin(virConnectPtr conn,
  asyncJob = QEMU_ASYNC_JOB_NONE;
  }
 
 +qemuDomainObjStorePreMigrationState(vm);
 +
  if (!virDomainObjIsActive(vm)  !(flags  VIR_MIGRATE_OFFLINE)) {
  virReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(domain is not running));
 @@ -2744,22 +2793,12 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
  /* cancel any outstanding NBD jobs */
  qemuMigrationCancelDriveMirror(mig, driver, vm);
 
 -/* run 'cont' on the destination, which allows migration on qemu
 - * = 0.10.6 to work properly.  This isn't strictly necessary on
 - * older qemu's, but it also doesn't hurt anything there
 - */
 -if (qemuProcessStartCPUs(driver, vm, conn,
 - VIR_DOMAIN_RUNNING_MIGRATED,
 - QEMU_ASYNC_JOB_MIGRATION_OUT)  0) {
 -if (virGetLastError() == NULL)
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 - 

Re: [libvirt] [PATCH V4] Sheepdog: Auto Adding volume and pool refresh.

[Daniel P. Berrange]

On Mon, Feb 03, 2014 at 05:55:49PM +0100, joel SIMOES wrote:
 From: Joel SIMOES joel.sim...@laposte.net
 
 Libvirt lose sheepdogs volumes on pool refresh or restart.
 When restarting sheepdog pool, all volumes are missing.
 This patch add automatically all volume from the added pool.
 
 With Jan Tomko help
 ---
  src/storage/storage_backend_sheepdog.c | 68 
 --
  1 file changed, 64 insertions(+), 4 deletions(-)
 
 diff --git a/src/storage/storage_backend_sheepdog.c 
 b/src/storage/storage_backend_sheepdog.c
 index a6981ce..7a139e7 100644
 --- a/src/storage/storage_backend_sheepdog.c
 +++ b/src/storage/storage_backend_sheepdog.c
 @@ -109,22 +109,82 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
  virCommandAddArgFormat(cmd, %d, port);
  }
  
 +static int
 +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
 +   virStoragePoolObjPtr pool)
 +{
 +int ret = -1;
 +char *output = NULL;
 +char** lines = NULL;
 +char** cells = NULL;

Nitpick - the '**' should associate with the variable name
rather than the type name.

 +size_t i;
 +
 +virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, -r, 
 NULL);
 +virStorageBackendSheepdogAddHostArg(cmd, pool);
 +virCommandSetOutputBuffer(cmd, output);
 +if (virCommandRun(cmd, NULL)  0)
 +goto cleanup;
 +
 +lines = virStringSplit(output, \n, 0);
 +if (lines == NULL)
 +goto cleanup;
 +
 +for (i = 0; lines[i]; i++) {
 +char *line = lines[i];
 +if (line == NULL)
 +break;
 +cells = virStringSplit(line,  , 0);
 +if (cells == NULL || virStringListLength(cells) = 2)
 +continue;

If cells != NULL, but the virStringListLength is = 2
then you leak the memory for 'cells'. You need to fre
cells before skiping to the next look iteration.

 +char *cell = cells[1];
 +virStorageVolDefPtr vol = NULL;
 +if (VIR_ALLOC(vol)  0)
 +goto cleanup;
 +
 +if (VIR_STRDUP(vol-name, cell)  0)
 +goto cleanup;

If this strdup fails, then this will leak memory
for 'vol', so need to have a free of that.

 +
 +vol-type = VIR_STORAGE_VOL_BLOCK;
 +
 +if (VIR_EXPAND_N(pool-volumes.objs, pool-volumes.count, 1)  0)
 +goto cleanup;
 +pool-volumes.objs[pool-volumes.count - 1] = vol;
 +
 +if (virStorageBackendSheepdogRefreshVol(conn, pool, vol)  0)
 +goto cleanup;
 +vol = NULL;
 +virStringFreeList(cells);

You need to assign 'cells = NULL' here, otherwise if
the loop terminates, you may get an attempt to free
the pointer/list again.

 +}
 +
 +ret = 0;
 +
 +cleanup:
 +virCommandFree(cmd);
 +virStringFreeList(lines);
 +virStringFreeList(cells);
 +return ret;
 +}


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 0/4] libxl: fixes related to concurrency improvements

[Jim Fehlig]

Michal Privoznik wrote:
 On 05.02.2014 18:39, Jim Fehlig wrote:
 While reviving old patches to add job support to the libxl driver,
 testing revealed some problems that were difficult to encounter
 in the current, more serialized processing approach used in the
 driver.

 The first patch is a bug fix, plugging leaks of libxlDomainObjPrivate
 objects.  The second patch removes the list of libxl timer registrations
 maintained in the driver - a hack I was never fond of.  The third patch
 moves domain shutdown handling to a thread, instead of doing all the
 shutdown work in the event handler.  The fourth patch fixes an issue wrt
 child process handling discussed in this thread

 http://lists.xen.org/archives/html/xen-devel/2014-01/msg01553.html

 Ian Jackson's latest patches on the libxl side are here

 http://lists.xen.org/archives/html/xen-devel/2014-02/msg00124.html


 Jim Fehlig (4):
libxl: fix leaking libxlDomainObjPrivate
libxl: remove list of timer registrations from libxlDomainObjPrivate
libxl: handle domain shutdown events in a thread
libxl: improve subprocess handling

   src/libxl/libxl_conf.h   |   5 +-
   src/libxl/libxl_domain.c | 102 ---
   src/libxl/libxl_domain.h |   8 +--
   src/libxl/libxl_driver.c | 135
 +++
   4 files changed, 115 insertions(+), 135 deletions(-)


 ACK series but see my comment on 3/4 where I'm asking for a pair of
 fixes prior pushing.

Thanks for pointing those out, especially creating the joinable thread
that was never joined :).  Fixed.  I also added a note to the commit
message of 4/4 stating that the fixes on the libxl side will be included
in Xen 4.4.0

http://lists.xen.org/archives/html/xen-devel/2014-02/msg00463.html

Pushed series.  Thanks!

Regards,
Jim

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

[libvirt] [PATCH] libxl: remove unneeded locking of driver when restoring

[Jim Fehlig]

libxlDomainRestoreFlags acquires the driver lock while reading the
domain config from the save file and adding it to
libxlDriverPrivatePtr-domains.  But virDomainObjList provides
self-locking APIs, so remove the needless driver locking.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---

I noticed this while working on the recent series to improve event handling
in the libxl driver, but it got lost when I reshuffled the patches a bit
before sending to the list.

 src/libxl/libxl_driver.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b852279..03aa463 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1956,11 +1956,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char 
*from,
 return -1;
 }
 
-/* Lock the driver until domain def is read from the saved
-   image and a virDomainObj is created and locked.
-*/
-libxlDriverLock(driver);
-
 fd = libxlSaveImageOpen(driver, cfg, from, def, hdr);
 if (fd  0)
 goto cleanup_unlock;
@@ -1975,7 +1970,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char 
*from,
NULL)))
 goto cleanup_unlock;
 
-libxlDriverUnlock(driver);
 def = NULL;
 
 ret = libxlVmStart(driver, vm, (flags  VIR_DOMAIN_SAVE_PAUSED) != 0, fd);
-- 
1.8.1.4

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

Re: [libvirt] [PATCH] libxl: remove unneeded locking of driver when restoring

[Michal Privoznik]

On 06.02.2014 18:46, Jim Fehlig wrote:

libxlDomainRestoreFlags acquires the driver lock while reading the
domain config from the save file and adding it to
libxlDriverPrivatePtr-domains.  But virDomainObjList provides
self-locking APIs, so remove the needless driver locking.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---

I noticed this while working on the recent series to improve event handling
in the libxl driver, but it got lost when I reshuffled the patches a bit
before sending to the list.

  src/libxl/libxl_driver.c | 6 --
  1 file changed, 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b852279..03aa463 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1956,11 +1956,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char 
*from,
  return -1;
  }

-/* Lock the driver until domain def is read from the saved
-   image and a virDomainObj is created and locked.
-*/
-libxlDriverLock(driver);
-
  fd = libxlSaveImageOpen(driver, cfg, from, def, hdr);
  if (fd  0)
  goto cleanup_unlock;
@@ -1975,7 +1970,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char 
*from,
 NULL)))
  goto cleanup_unlock;

-libxlDriverUnlock(driver);
  def = NULL;

  ret = libxlVmStart(driver, vm, (flags  VIR_DOMAIN_SAVE_PAUSED) != 0, fd);



Right, this locking is not necessary. ACK

Michal

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

[libvirt] [PATCH 0/2] glusterfs related fixes

[Christophe Fergeau]

Hi,

I noticed that virsh pool-list --type gluster is not giving me an empty
list as expected, but rather the list of active pools. This is happening because
virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because
VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized.

vshStoragePoolListCollect then goes to retry with only
VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE
in the flags, which is questionable, but explains the result I got.

Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to 
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE
is enough to fix that.

I've played a bit with adding some compile time checks to avoid this in the 
future, but
while they work, they are a bit ugly-looking, and they would require some 
changes in
docs/apibuild.py to teach it to parse 1  N, or to make it ignore values
in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd 
like to know
if these checks would be a welcome/useful addition.

The checks are:

commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d
Author: Christophe Fergeau cferg...@redhat.com
Date:   Thu Feb 6 17:39:43 2014 +0100

add pool related compile-time checks

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b3ce000..9d79ae6 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3104,6 +3104,10 @@ typedef enum {
 VIR_CONNECT_LIST_STORAGE_POOLS_RBD   = 1  14,
 VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG  = 1  15,
 VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER   = 1  16,
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_CONNECT_LIST_STORAGE_POOLS_LAST
+#endif
 } virConnectListAllStoragePoolsFlags;
 
 int virConnectListAllStoragePools(virConnectPtr conn,
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index eaa9325..c8d4df4 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -36,6 +36,7 @@
 #include virerror.h
 #include datatypes.h
 #include storage_conf.h
+#include verify.h
 #include virstoragefile.h
 
 #include virxml.h
@@ -51,6 +52,16 @@
 #define DEFAULT_POOL_PERM_MODE 0755
 #define DEFAULT_VOL_PERM_MODE  0600
 
+/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to
+ * contain all virConnectListAllStoragePoolsFlags elements
+ */
+verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST  
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+/* Check if virConnectListAllStorageFlags has one flag per known storage
+ * pool type */
+verify(1  (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 
1);
+
+
 VIR_ENUM_IMPL(virStorageVol,
   VIR_STORAGE_VOL_LAST,
   file, block, dir, network, netdir)
@@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
 }
 };
 
+/* Check if poolTypeInfo has one entry per known storage pool type */
+verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST);
 
 static virStoragePoolTypeInfoPtr
 virStoragePoolTypeInfoLookup(int type)

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

[libvirt] [PATCH v2 0/2] small gluster fixes

[Christophe Fergeau]

Hi,

I noticed that virsh pool-list --type gluster is not giving me an empty
list as expected, but rather the list of active pools. This is happening because
virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because
VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized.

vshStoragePoolListCollect then goes to retry with only
VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE
in the flags, which is questionable, but explains the result I got.

Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to 
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE
is enough to fix that.

I've played a bit with adding some compile time checks to avoid this in the 
future, but
while they work, they are a bit ugly-looking, and they would require some 
changes in
docs/apibuild.py to teach it to parse 1  N, or to make it ignore values
in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd 
like to know
if these checks would be a welcome/useful addition.

The checks are:

commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d
Author: Christophe Fergeau cferg...@redhat.com
Date:   Thu Feb 6 17:39:43 2014 +0100

add pool related compile-time checks

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b3ce000..9d79ae6 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3104,6 +3104,10 @@ typedef enum {
 VIR_CONNECT_LIST_STORAGE_POOLS_RBD   = 1  14,
 VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG  = 1  15,
 VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER   = 1  16,
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_CONNECT_LIST_STORAGE_POOLS_LAST
+#endif
 } virConnectListAllStoragePoolsFlags;
 
 int virConnectListAllStoragePools(virConnectPtr conn,
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index eaa9325..c8d4df4 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -36,6 +36,7 @@
 #include virerror.h
 #include datatypes.h
 #include storage_conf.h
+#include verify.h
 #include virstoragefile.h
 
 #include virxml.h
@@ -51,6 +52,16 @@
 #define DEFAULT_POOL_PERM_MODE 0755
 #define DEFAULT_VOL_PERM_MODE  0600
 
+/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to
+ * contain all virConnectListAllStoragePoolsFlags elements
+ */
+verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST  
VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+/* Check if virConnectListAllStorageFlags has one flag per known storage
+ * pool type */
+verify(1  (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 
1);
+
+
 VIR_ENUM_IMPL(virStorageVol,
   VIR_STORAGE_VOL_LAST,
   file, block, dir, network, netdir)
@@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
 }
 };
 
+/* Check if poolTypeInfo has one entry per known storage pool type */
+verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST);
 
 static virStoragePoolTypeInfoPtr
 virStoragePoolTypeInfoLookup(int type)

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

[libvirt] [PATCH 1/2 1/3] Add glusterfs to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE

[Christophe Fergeau]

If it's not present in this list, we won't be able to get only
glusterfs pools when using virConnectListAllStoragePools.
---
 src/conf/storage_conf.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 62ac749..cada861 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -575,7 +575,8 @@ VIR_ENUM_DECL(virStoragePartedFsType)
  VIR_CONNECT_LIST_STORAGE_POOLS_SCSI | \
  VIR_CONNECT_LIST_STORAGE_POOLS_MPATH| \
  VIR_CONNECT_LIST_STORAGE_POOLS_RBD  | \
- VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG)
+ VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG | \
+ VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER)
 
 # define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL  \
 (VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE | \
-- 
1.8.5.3

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

Re: [libvirt] [PATCH 0/2] glusterfs related fixes

[Christophe Fergeau]

Sorry, screwed up sending the patches, will send a v2, ignore this thread.

Christophe

On Thu, Feb 06, 2014 at 07:03:25PM +0100, Christophe Fergeau wrote:
 Hi,
 
 I noticed that virsh pool-list --type gluster is not giving me an empty
 list as expected, but rather the list of active pools. This is happening 
 because
 virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because
 VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized.
 
 vshStoragePoolListCollect then goes to retry with only
 VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | 
 VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE
 in the flags, which is questionable, but explains the result I got.
 
 Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to 
 VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE
 is enough to fix that.
 
 I've played a bit with adding some compile time checks to avoid this in the 
 future, but
 while they work, they are a bit ugly-looking, and they would require some 
 changes in
 docs/apibuild.py to teach it to parse 1  N, or to make it ignore values
 in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd 
 like to know
 if these checks would be a welcome/useful addition.
 
 The checks are:
 
 commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d
 Author: Christophe Fergeau cferg...@redhat.com
 Date:   Thu Feb 6 17:39:43 2014 +0100
 
 add pool related compile-time checks
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index b3ce000..9d79ae6 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -3104,6 +3104,10 @@ typedef enum {
  VIR_CONNECT_LIST_STORAGE_POOLS_RBD   = 1  14,
  VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG  = 1  15,
  VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER   = 1  16,
 +
 +#ifdef VIR_ENUM_SENTINELS
 +VIR_CONNECT_LIST_STORAGE_POOLS_LAST
 +#endif
  } virConnectListAllStoragePoolsFlags;
  
  int virConnectListAllStoragePools(virConnectPtr conn,
 diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
 index eaa9325..c8d4df4 100644
 --- a/src/conf/storage_conf.c
 +++ b/src/conf/storage_conf.c
 @@ -36,6 +36,7 @@
  #include virerror.h
  #include datatypes.h
  #include storage_conf.h
 +#include verify.h
  #include virstoragefile.h
  
  #include virxml.h
 @@ -51,6 +52,16 @@
  #define DEFAULT_POOL_PERM_MODE 0755
  #define DEFAULT_VOL_PERM_MODE  0600
  
 +/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to
 + * contain all virConnectListAllStoragePoolsFlags elements
 + */
 +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST  
 VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
 +
 +/* Check if virConnectListAllStorageFlags has one flag per known storage
 + * pool type */
 +verify(1  (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST 
 - 1);
 +
 +
  VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
file, block, dir, network, netdir)
 @@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
  }
  };
  
 +/* Check if poolTypeInfo has one entry per known storage pool type */
 +verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST);
  
  static virStoragePoolTypeInfoPtr
  virStoragePoolTypeInfoLookup(int type)
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


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

[libvirt] [PATCH v2 1/2] build-sys: Removed unused variable from configure.ac

[Christophe Fergeau]

LIBGLUSTER_LIBS is emptied before gluster is enabled/disabled, but nothing
else sets/uses this variable, so it can be removed.
---
 configure.ac | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 884e0e4..e233706 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1907,7 +1907,6 @@ fi
 AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG],
   [test $with_storage_sheepdog = yes])
 
-LIBGLUSTER_LIBS=
 if test $with_storage_gluster = check; then
   with_storage_gluster=$with_glusterfs
 fi
-- 
1.8.5.3

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

[libvirt] [PATCH v2 2/2] Add glusterfs to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE

[Christophe Fergeau]

If it's not present in this list, we won't be able to get only
glusterfs pools when using virConnectListAllStoragePools.
---
 src/conf/storage_conf.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 62ac749..cada861 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -575,7 +575,8 @@ VIR_ENUM_DECL(virStoragePartedFsType)
  VIR_CONNECT_LIST_STORAGE_POOLS_SCSI | \
  VIR_CONNECT_LIST_STORAGE_POOLS_MPATH| \
  VIR_CONNECT_LIST_STORAGE_POOLS_RBD  | \
- VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG)
+ VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG | \
+ VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER)
 
 # define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL  \
 (VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE | \
-- 
1.8.5.3

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

Re: [libvirt] [PATCH] libxl: remove unneeded locking of driver when restoring

[Jim Fehlig]

Michal Privoznik wrote:
 On 06.02.2014 18:46, Jim Fehlig wrote:
 libxlDomainRestoreFlags acquires the driver lock while reading the
 domain config from the save file and adding it to
 libxlDriverPrivatePtr-domains.  But virDomainObjList provides
 self-locking APIs, so remove the needless driver locking.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---

 I noticed this while working on the recent series to improve event
 handling
 in the libxl driver, but it got lost when I reshuffled the patches a bit
 before sending to the list.

   src/libxl/libxl_driver.c | 6 --
   1 file changed, 6 deletions(-)

 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index b852279..03aa463 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -1956,11 +1956,6 @@ libxlDomainRestoreFlags(virConnectPtr conn,
 const char *from,
   return -1;
   }

 -/* Lock the driver until domain def is read from the saved
 -   image and a virDomainObj is created and locked.
 -*/
 -libxlDriverLock(driver);
 -
   fd = libxlSaveImageOpen(driver, cfg, from, def, hdr);
   if (fd  0)
   goto cleanup_unlock;
 @@ -1975,7 +1970,6 @@ libxlDomainRestoreFlags(virConnectPtr conn,
 const char *from,
  NULL)))
   goto cleanup_unlock;

 -libxlDriverUnlock(driver);
   def = NULL;

   ret = libxlVmStart(driver, vm, (flags  VIR_DOMAIN_SAVE_PAUSED)
 != 0, fd);


 Right, this locking is not necessary. ACK

Thanks, pushed.

Regards,
Jim

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

Re: [libvirt] [PATCH] qemu: keep pre-migration domain state after failed migration

[Eric Blake]

On 02/06/2014 09:33 AM, Martin Kletzander wrote:
 Couple of codepaths shared the same code which can be moved out to a
 function and on one of such places, qemuMigrationConfirmPhase(), the
 domain was resumed even if it wasn't running before the migration
 started.
 

 +if (state == VIR_DOMAIN_PAUSED 
 +priv-preMigrationState == VIR_DOMAIN_RUNNING) {
 +/* This is basically the only restore possibility that's safe
 + * and we should attemt to do */

s/attemt/attempt/


-- 
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/2] Add glusterfs to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE

[Eric Blake]

On 02/06/2014 11:09 AM, Christophe Fergeau wrote:
 If it's not present in this list, we won't be able to get only
 glusterfs pools when using virConnectListAllStoragePools.
 ---
  src/conf/storage_conf.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
 index 62ac749..cada861 100644
 --- a/src/conf/storage_conf.h
 +++ b/src/conf/storage_conf.h
 @@ -575,7 +575,8 @@ VIR_ENUM_DECL(virStoragePartedFsType)
   VIR_CONNECT_LIST_STORAGE_POOLS_SCSI | \
   VIR_CONNECT_LIST_STORAGE_POOLS_MPATH| \
   VIR_CONNECT_LIST_STORAGE_POOLS_RBD  | \
 - VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG)
 + VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG | \
 + VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER)

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/2] build-sys: Removed unused variable from configure.ac

[Eric Blake]

On 02/06/2014 11:09 AM, Christophe Fergeau wrote:
 LIBGLUSTER_LIBS is emptied before gluster is enabled/disabled, but nothing
 else sets/uses this variable, so it can be removed.
 ---
  configure.ac | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/configure.ac b/configure.ac
 index 884e0e4..e233706 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1907,7 +1907,6 @@ fi
  AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG],
[test $with_storage_sheepdog = yes])
  
 -LIBGLUSTER_LIBS=
  if test $with_storage_gluster = check; then

ACK.  Leftovers from my earlier drafts.

-- 
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 0/2] small gluster fixes

[Eric Blake]

On 02/06/2014 11:09 AM, Christophe Fergeau wrote:

 
 I've played a bit with adding some compile time checks to avoid this in the 
 future, but
 while they work, they are a bit ugly-looking, and they would require some 
 changes in
 docs/apibuild.py to teach it to parse 1  N, or to make it ignore values
 in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd 
 like to know
 if these checks would be a welcome/useful addition.

Hmm, we don't add pools every day.


  
 +/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to
 + * contain all virConnectListAllStoragePoolsFlags elements
 + */
 +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST  
 VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
 +
 +/* Check if virConnectListAllStorageFlags has one flag per known storage
 + * pool type */
 +verify(1  (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST 
 - 1);

Wow, that does look pretty ugly and hard-coded.  At this point, I'm not
sure that adding it buys us much.

-- 
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 7/6] qemu: allow filtering events by regex

[Eric Blake]

When listening for a subset of monitor events, it can be tedious
to register for each event name in series; nicer is to register
for multiple events in one go.  Implement a flag to use regex
interpretation of the event filter.

While at it, prove how much I hate the shift key, by adding a
way to filter for 'shutdown' instead of 'SHUTDOWN'. :)

* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventRegisterFlags): New enum.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister):
Document flags.
* tools/virsh-domain.c (cmdQemuMonitorEvent): Expose them.
* tools/virsh.pod (qemu-monitor-event): Document this.
* src/conf/domain_event.h
(virDomainQemuMonitorEventStateRegisterID): Adjust signature.
* src/conf/domain_event.c
(virDomainQemuMonitorEventStateRegisterID): Add flags.
(virDomainQemuMonitorEventFilter): Handle regex, and optimize
client side.
(virDomainQemuMonitorEventCleanup): Clean up regex.
* src/qemu/qemu_driver.c
(qemuConnectDomainQemuMonitorEventRegister): Adjust caller.
* src/remote/remote_driver.c
(remoteConnectDomainQemuMonitorEventRegister): New flags can
always be safely handled server side.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 include/libvirt/libvirt-qemu.h | 10 ++
 src/conf/domain_event.c| 41 ++---
 src/conf/domain_event.h|  3 ++-
 src/libvirt-qemu.c | 11 +++
 src/qemu/qemu_driver.c |  7 +--
 src/remote/remote_driver.c |  8 +---
 tools/virsh-domain.c   | 13 +
 tools/virsh.pod|  5 -
 8 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index ed6d3d2..652926e 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -76,6 +76,16 @@ typedef void 
(*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn,
  const char *details,
  void *opaque);

+
+typedef enum {
+/* Event filter is a regex rather than a literal string */
+VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX = (1  0),
+
+/* Event filter is case insensitive */
+VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE = (1  1),
+} virConnectDomainQemuMonitorEventRegisterFlags;
+
+
 int virConnectDomainQemuMonitorEventRegister(virConnectPtr conn,
  virDomainPtr dom,
  const char *event,
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 1fb5243..ab468c9 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -24,6 +24,8 @@

 #include config.h

+#include regex.h
+
 #include domain_event.h
 #include object_event.h
 #include object_event_private.h
@@ -1389,6 +1391,8 @@ error:
  * deregisters.  */
 struct virDomainQemuMonitorEventData {
 char *event;
+regex_t regex;
+unsigned int flags;
 void *opaque;
 virFreeCallback freecb;
 };
@@ -1608,6 +1612,12 @@ virDomainQemuMonitorEventFilter(virConnectPtr conn 
ATTRIBUTE_UNUSED,

 monitorEvent = (virDomainQemuMonitorEventPtr) event;

+if (data-flags == -1)
+return true;
+if (data-flags  VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
+return regexec(data-regex, monitorEvent-event, 0, NULL, 0) == 0;
+if (data-flags  VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
+return STRCASEEQ(monitorEvent-event, data-event);
 return STREQ(monitorEvent-event, data-event);
 }

@@ -1618,6 +1628,8 @@ virDomainQemuMonitorEventCleanup(void *opaque)
 virDomainQemuMonitorEventData *data = opaque;

 VIR_FREE(data-event);
+if (data-flags  VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
+regfree(data-regex);
 if (data-freecb)
 (data-freecb)(data-opaque);
 VIR_FREE(data);
@@ -1633,6 +1645,8 @@ virDomainQemuMonitorEventCleanup(void *opaque)
  * @cb: function to invoke when event occurs
  * @opaque: data blob to pass to callback
  * @freecb: callback to free @opaque
+ * @flags: -1 for client, valid virConnectDomainQemuMonitorEventRegisterFlags
+ * for server
  * @callbackID: filled with callback ID
  *
  * Register the function @cb with connection @conn, from @state, for
@@ -1648,6 +1662,7 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr 
conn,
  
virConnectDomainQemuMonitorEventCallback cb,
  void *opaque,
  virFreeCallback freecb,
+ unsigned int flags,
  int *callbackID)
 {
 virDomainQemuMonitorEventData *data = NULL;
@@ -1658,9 +1673,29 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr 
conn,

 if (VIR_ALLOC(data)  0)
 return -1;
-if 

Re: [libvirt] [PATCHv4 1/7] storage: gluster: Set volume metadata in a separate function

[Eric Blake]

On 02/03/2014 09:54 AM, Peter Krempa wrote:
 Extract the metadata setting code into a separate function for future
 use.
 ---
  src/storage/storage_backend_gluster.c | 66 
 ---
  1 file changed, 46 insertions(+), 20 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] Skip the 'copyright-check' on -maint branches

[Eric Blake]

On 02/04/2014 06:51 AM, Daniel P. Berrange wrote:
 The maint branches will often have out of date copyright headers
 so we must skip the 'sc_copyright_check' rule there.

Is that the only rule, or are there others?  But yeah, that's definitely
the most annoying one.

 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  cfg.mk | 11 +++
  1 file changed, 11 insertions(+)
 
 Perhaps there's a better way to detect -maint branches than the
 logic I tried.
 
 diff --git a/cfg.mk b/cfg.mk
 index 207dfeb..47c7798 100644
 --- a/cfg.mk
 +++ b/cfg.mk
 @@ -88,6 +88,17 @@ else
  distdir: sc_vulnerable_makefile_CVE-2012-3386.z
  endif
  
 +GIT_BRANCH := $(shell if test -d $(srcdir)/.git; then \
 + cd $(srcdir)  \
 + (git status -s -b | grep '\#\#' | \
 + sed -e 's/.*-maint/-maint/'); fi)

I'm still thinking about this one...

 +
 +# We fully expect -maint branches to have out of date
 +# copyright dates, so we must skip that check
 +ifeq ($(GIT_BRANCH),-maint)
 +local-checks-to-skip += sc_copyright_check
 +endif

But this part looks fine, once the GIT_BRANCH rule is optimized.

 +
  # Files that should never cause syntax check failures.
  VC_LIST_ALWAYS_EXCLUDE_REGEX = \
(^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$
 

-- 
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 v2] libvirt support to force convergence of live guest migration

[Chegu Vinod]

Hello,
'am sending this updated patch as an attachment (as I was having some 
issues with my smtp server and git send-email setup).

Thanks,
Vinod


Subject: [PATCH v2] libvirt support to force convergence of live guest migration

Changes since RFC (i.e. v1) patch :
- Incorporated feedback from Jiri Denemark

Busy enterprise workloads hosted on large sized VM's tend to dirty
memory faster than the transfer rate achieved via live guest migration.
Despite some good recent improvements ( using dedicated 10Gig NICs
between hosts) the live migration may NOT converge.

Recently support was added in qemu (version 1.6) to allow a user to
choose if they wish to force convergence of their migration via a
new migration capability : auto-converge. This feature allows for qemu
to auto-detect lack of convergence and trigger a throttle-down of the
VCPUs.

This patch includes the libvirt support needed to trigger this
feature. (Testing is in progress)

Signed-off-by:  Chegu Vinod chegu_vi...@hp.com
---
 include/libvirt/libvirt.h.in |  1 +
 src/qemu/qemu_migration.c| 40 
 src/qemu/qemu_migration.h|  3 ++-
 src/qemu/qemu_monitor.c  |  2 +-
 src/qemu/qemu_monitor.h  |  1 +
 tools/virsh-domain.c |  7 +++
 6 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 295d551..c0fc08b 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1200,6 +1200,7 @@ typedef enum {
 VIR_MIGRATE_OFFLINE   = (1  10), /* offline migrate */
 VIR_MIGRATE_COMPRESSED= (1  11), /* compress data during 
migration */
 VIR_MIGRATE_ABORT_ON_ERROR= (1  12), /* abort migration on I/O 
errors happened during migration */
+VIR_MIGRATE_AUTO_CONVERGE = (1  13), /* force convergence */
 } virDomainMigrateFlags;
 
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 407fb70..379d19b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1565,6 +1565,41 @@ cleanup:
 }
 
 static int
+qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ enum qemuDomainAsyncJob job)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job)  0)
+return -1;
+
+ret = qemuMonitorGetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
+
+if (ret  0) {
+goto cleanup;
+} else if (ret == 0) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(Auto-Converge is not supported by 
+ QEMU binary));
+ret = -1;
+goto cleanup;
+}
+
+ret = qemuMonitorSetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE);
+
+cleanup:
+qemuDomainObjExitMonitor(driver, vm);
+return ret;
+}
+
+
+static int
 qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
   virDomainObjPtr vm)
 {
@@ -3175,6 +3210,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;
 
+if (flags  VIR_MIGRATE_AUTO_CONVERGE 
+qemuMigrationSetAutoConverge(driver, vm,
+ QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
+goto cleanup;
+
 if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index cafa2a2..a802fb7 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -39,7 +39,8 @@
  VIR_MIGRATE_UNSAFE |   \
  VIR_MIGRATE_OFFLINE |  \
  VIR_MIGRATE_COMPRESSED |   \
- VIR_MIGRATE_ABORT_ON_ERROR)
+ VIR_MIGRATE_ABORT_ON_ERROR |   \
+ VIR_MIGRATE_AUTO_CONVERGE)
 
 /* All supported migration parameters and their types. */
 # define QEMU_MIGRATION_PARAMETERS  \
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a2769db..201f60c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -118,7 +118,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
 
 VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
   QEMU_MONITOR_MIGRATION_CAPS_LAST,
-  xbzrle)
+  xbzrle, auto-converge)
 
 VIR_ENUM_IMPL(qemuMonitorVMStatus,
   QEMU_MONITOR_VM_STATUS_LAST,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index eabf000..95e70ab 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -440,6 +440,7 @@ int qemuMonitorGetSpiceMigrationStatus(qemuMonitorPtr mon,
 
 typedef enum {
 QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
+

Re: [libvirt] [PATCHv4 2/7] storage: Add APIs for internal handling of files via the storage driver

[Eric Blake]

On 02/03/2014 09:54 AM, Peter Krempa wrote:
 Some remote filesystems are not accessible via the local filesystem
 calls, but libvirt needs means to do operations on such files.
 
 This patch adds internal APIs into the storage driver that will allow
 operations on various networked and other filesystems.
 ---
  docs/hvsupport.pl|   4 +-
  po/POTFILES.in   |   1 +
  src/Makefile.am  |   1 +
  src/check-drivername.pl  |   1 +
  src/driver.h |  30 +++-
  src/libvirt_private.c| 176 
 +++
  src/libvirt_private.h|  61 
  src/libvirt_private.syms |   9 +++
  8 files changed, 281 insertions(+), 2 deletions(-)
  create mode 100644 src/libvirt_private.c
  create mode 100644 src/libvirt_private.h
 

 +++ b/src/Makefile.am
 @@ -173,6 +173,7 @@ DRIVER_SOURCES =  
 \
   fdstream.c fdstream.h   \
   $(NODE_INFO_SOURCES)\
   libvirt.c libvirt_internal.h\
 + libvirt_private.h libvirt_private.c \
   locking/lock_manager.c locking/lock_manager.h   \

Alignment of \ looks off.


 +virStorageFilePtr
 +virStorageFileInitFromDiskDef(virConnectPtr conn,
 +  virDomainDiskDefPtr disk)
 +{

 +
 +if (file-driver-storageFileInit) {
 +if (file-driver-storageFileInit(file)  0)

This could be joined with  instead of nested if.


 +
 +
 +virStorageFilePtr
 +virStorageFileInitFromSnapshotDef(virConnectPtr conn,
 +  virDomainSnapshotDiskDefPtr disk)

 +
 +if (file-driver-storageFileInit) {
 +if (file-driver-storageFileInit(file)  0)
 +goto error;

Same here.

 +/**
 + * virStorageFileUnlink: Unlink storage file via storage driver
 + *
 + * @file: file structure poiniting to the file

s/poiniting/pointing/

Those are all minor.  Wait for the review of the full series before
pushing this, but I'm okay with ACK for htis one.

-- 
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 0/3] libxl: misc trivial cleanups

[Jim Fehlig]

A few trivial patches for things I noticed while working on my
upcoming patches that add job support in the libxl driver.

Jim Fehlig (3):
  libxl: rename libxlCreateDomEvents to libxlDomEventsRegister
  libxl: register for domain events immediately after creation
  libxl: fix libxlDoDomainSave documentation

 src/libxl/libxl_driver.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
1.8.1.4

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

[libvirt] [PATCH 1/3] libxl: rename libxlCreateDomEvents to libxlDomEventsRegister

[Jim Fehlig]

libxlDomEventsRegister better reflects its purpose: register for
domain events from libxl.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 03aa463..50fbe5c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -464,16 +464,16 @@ static const struct libxl_event_hooks ev_hooks = {
 };
 
 /*
- * Register domain events with libxenlight and insert event handles
- * in libvirt's event loop.
+ * Register for domain events emitted by libxl.
  */
 static int
-libxlCreateDomEvents(virDomainObjPtr vm)
+libxlDomEventsRegister(virDomainObjPtr vm)
 {
 libxlDomainObjPrivatePtr priv = vm-privateData;
 
 libxl_event_register_callbacks(priv-ctx, ev_hooks, vm);
 
+/* Always enable domain death events */
 if (libxl_evenable_domain_death(priv-ctx, vm-def-id, 0, priv-deathW))
 goto error;
 
@@ -700,7 +700,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
 goto error;
 }
 
-if (libxlCreateDomEvents(vm)  0)
+if (libxlDomEventsRegister(vm)  0)
 goto error;
 
 if (libxlDomainSetVcpuAffinities(driver, vm)  0)
@@ -791,8 +791,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
 if (virAtomicIntInc(driver-nactive) == 1  driver-inhibitCallback)
 driver-inhibitCallback(true, driver-inhibitOpaque);
 
-/* Recreate domain death et. al. events */
-libxlCreateDomEvents(vm);
+/* Re-register domain death et. al. events */
+libxlDomEventsRegister(vm);
 virObjectUnlock(vm);
 return 0;
 
-- 
1.8.1.4

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

[libvirt] [PATCH 2/3] libxl: register for domain events immediately after creation

[Jim Fehlig]

A small fix for the possiblitiy of jumping to an error path before
registering for domain events, preventing receiving important ones
like shutdown and death.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 50fbe5c..99643e3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -690,6 +690,9 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
 }
 
 vm-def-id = domid;
+if (libxlDomEventsRegister(vm)  0)
+goto error;
+
 if ((dom_xml = virDomainDefFormat(vm-def, 0)) == NULL)
 goto error;
 
@@ -700,9 +703,6 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
 goto error;
 }
 
-if (libxlDomEventsRegister(vm)  0)
-goto error;
-
 if (libxlDomainSetVcpuAffinities(driver, vm)  0)
 goto error;
 
-- 
1.8.1.4

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

[libvirt] [PATCH 3/3] libxl: fix libxlDoDomainSave documentation

[Jim Fehlig]

Update the function's comment, which was missed when removing use of
the driver lock everywhere.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 99643e3..8e4242a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1812,8 +1812,9 @@ libxlDomainGetState(virDomainPtr dom,
 return ret;
 }
 
-/* This internal function expects the driver lock to already be held on
- * entry and the vm must be active. */
+/*
+ * virDomainObjPtr must be locked on invocation
+ */
 static int
 libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
   const char *to)
-- 
1.8.1.4

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

[libvirt] [PATCH 00/11] libxl: add job support

[Jim Fehlig]

This patch series adds job support to the libxl driver, using techiques from
the qemu driver.  One benefit is no longer blocking get operations during
long running modify operations.  E.g. with these patches 'vish dominfo dom'
will work while 'virsh save dom ...' is in progress.

The first patch adds the job support machinery, followed by several patches
that make use of it.  I initially had all but the first in a single
use-job-support patch, but hope breaking it out eases review.

Jim Fehlig (11):
  libxl: Add job support to libxl driver
  libxl: use job functions in libxlVmStart
  libxl: use job functions in libxlDomainSetMemoryFlags
  libxl: use job functions in libxlDomain{Suspend,Resume}
  libxl: use job functions in libxlDomainDestroyFlags
  libxl: use job functions in domain save operations
  libxl: use job functions in libxlDomainCoreDump
  libxl: use job functions in vcpu set and pin functions
  libxl: use job functions in device attach and detach functions
  libxl: use job functions in libxlDomainSetAutostart
  libxl: use job functions in libxlDomainSetSchedulerParametersFlags

 src/libxl/libxl_domain.c | 128 ++
 src/libxl/libxl_domain.h |  37 +
 src/libxl/libxl_driver.c | 346 +++
 3 files changed, 395 insertions(+), 116 deletions(-)

-- 
1.8.1.4

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

[libvirt] [PATCH 08/11] libxl: use job functions in vcpu set and pin functions

[Jim Fehlig]

These operations aren't necessarily time consuming, but need to
wait in the queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 46 +-
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 84d9ca3..7ce127a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2322,22 +2322,25 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 if (virDomainSetVcpusFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)  (flags  VIR_DOMAIN_VCPU_LIVE)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(cannot set vcpus on an inactive domain));
-goto cleanup;
+goto endjob;
 }
 
 if (!vm-persistent  (flags  VIR_DOMAIN_VCPU_CONFIG)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(cannot change persistent config of a transient 
domain));
-goto cleanup;
+goto endjob;
 }
 
 if ((max = libxlConnectGetMaxVcpus(dom-conn, NULL))  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(could not determine max vcpus for the domain));
-goto cleanup;
+goto endjob;
 }
 
 if (!(flags  VIR_DOMAIN_VCPU_MAXIMUM)  vm-def-maxvcpus  max) {
@@ -2348,17 +2351,17 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 virReportError(VIR_ERR_INVALID_ARG,
_(requested vcpus is greater than max allowable
   vcpus for the domain: %d  %d), nvcpus, max);
-goto cleanup;
+goto endjob;
 }
 
 priv = vm-privateData;
 
 if (!(def = virDomainObjGetPersistentDef(cfg-caps, driver-xmlopt, vm)))
-goto cleanup;
+goto endjob;
 
 maplen = VIR_CPU_MAPLEN(nvcpus);
 if (VIR_ALLOC_N(bitmask, maplen)  0)
-goto cleanup;
+goto endjob;
 
 for (i = 0; i  nvcpus; ++i) {
 pos = i / 8;
@@ -2384,7 +2387,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to set vcpus for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 break;
 
@@ -2393,7 +2396,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to set vcpus for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 def-vcpus = nvcpus;
 break;
@@ -2404,6 +2407,9 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 if (flags  VIR_DOMAIN_VCPU_CONFIG)
 ret = virDomainSaveConfig(cfg-configDir, def);
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 VIR_FREE(bitmask);
  if (vm)
@@ -2495,15 +2501,18 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 if (virDomainPinVcpuFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if ((flags  VIR_DOMAIN_AFFECT_LIVE)  !virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(domain is inactive));
-goto cleanup;
+goto endjob;
 }
 
 if (virDomainLiveConfigHelperMethod(cfg-caps, driver-xmlopt, vm,
 flags, targetDef)  0)
-goto cleanup;
+goto endjob;
 
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 targetDef = vm-def;
@@ -2514,7 +2523,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 
 pcpumap = virBitmapNewData(cpumap, maplen);
 if (!pcpumap)
-goto cleanup;
+goto endjob;
 
 if (flags  VIR_DOMAIN_AFFECT_LIVE) {
 libxl_bitmap map = { .size = maplen, .map = cpumap };
@@ -2525,7 +2534,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to pin vcpu '%d' with libxenlight),
vcpu);
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -2535,14 +2544,14 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to delete vcpupin xml for vcpu '%d'),
vcpu);
-goto cleanup;
+goto endjob;
 }
-goto out;
+goto done;
 }
 
 if (!targetDef-cputune.vcpupin) {
 if 

[libvirt] [PATCH 02/11] libxl: use job functions in libxlVmStart

[Jim Fehlig]

Creating a large domain could potentially be time consuming.  Use the
recently added job functions and unlock the virDomainObj while
the create operation is in progress.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 59 ++--
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8e4242a..7c964c5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -593,7 +593,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
 virDomainDefPtr def = NULL;
 virObjectEventPtr event = NULL;
 libxlSavefileHeader hdr;
-int ret;
+int ret = -1;
 uint32_t domid = 0;
 char *dom_xml = NULL;
 char *managed_save_path = NULL;
@@ -605,14 +605,17 @@ libxlVmStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 #endif
 
 if (libxlDomainObjPrivateInitCtx(vm)  0)
-goto error;
+goto cleanup;
+
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
 
 /* If there is a managed saved state restore it instead of starting
  * from scratch. The old state is removed once the restoring succeeded. */
 if (restore_fd  0) {
 managed_save_path = libxlDomainManagedSavePath(driver, vm);
 if (managed_save_path == NULL)
-goto error;
+goto endjob;
 
 if (virFileExists(managed_save_path)) {
 
@@ -620,7 +623,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
  managed_save_path,
  def, hdr);
 if (managed_save_fd  0)
-goto error;
+goto endjob;
 
 restore_fd = managed_save_fd;
 
@@ -634,7 +637,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
_(cannot restore domain '%s' uuid %s from a 
file
   which belongs to domain '%s' uuid %s),
vm-def-name, vm_uuidstr, def-name, 
def_uuidstr);
-goto error;
+goto endjob;
 }
 
 virDomainObjAssignDef(vm, def, true, NULL);
@@ -652,17 +655,17 @@ libxlVmStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 libxl_domain_config_init(d_config);
 
 if (libxlBuildDomainConfig(driver, vm, d_config)  0)
-goto error;
+goto endjob;
 
 if (cfg-autoballoon  libxlFreeMem(priv, d_config)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(libxenlight failed to get free memory for domain 
'%s'),
d_config.c_info.name);
-goto error;
+goto endjob;
 }
 
-/* use as synchronous operations = ao_how = NULL and no intermediate 
reports = ao_progress = NULL */
-
+/* Unlock virDomainObj while creating the domain */
+virObjectUnlock(vm);
 if (restore_fd  0) {
 ret = libxl_domain_create_new(priv-ctx, d_config,
   domid, NULL, NULL);
@@ -676,6 +679,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
   restore_fd, NULL, NULL);
 #endif
 }
+virObjectLock(vm);
 
 if (ret) {
 if (restore_fd  0)
@@ -686,25 +690,25 @@ libxlVmStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(libxenlight failed to restore domain '%s'),
d_config.c_info.name);
-goto error;
+goto endjob;
 }
 
 vm-def-id = domid;
 if (libxlDomEventsRegister(vm)  0)
-goto error;
+goto endjob;
 
 if ((dom_xml = virDomainDefFormat(vm-def, 0)) == NULL)
-goto error;
+goto endjob;
 
 if (libxl_userdata_store(priv-ctx, domid, libvirt-xml,
  (uint8_t *)dom_xml, strlen(dom_xml) + 1)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(libxenlight failed to store userdata));
-goto error;
+goto endjob;
 }
 
 if (libxlDomainSetVcpuAffinities(driver, vm)  0)
-goto error;
+goto endjob;
 
 if (!start_paused) {
 libxl_domain_unpause(priv-ctx, domid);
@@ -715,7 +719,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr 
vm,
 
 
 if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
-goto error;
+goto endjob;
 
 if (virAtomicIntInc(driver-nactive) == 1  driver-inhibitCallback)
 driver-inhibitCallback(true, driver-inhibitOpaque);
@@ -727,25 +731,26 @@ libxlVmStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 if (event)
 libxlDomainEventQueue(driver, event);
 
-libxl_domain_config_dispose(d_config);
-VIR_FREE(dom_xml);
-VIR_FORCE_CLOSE(managed_save_fd);
-

[libvirt] [PATCH 03/11] libxl: use job functions in libxlDomainSetMemoryFlags

[Jim Fehlig]

Large balloon operation can be time consuming.  Use the recently
added job functions and unlock the virDomainObj while ballooning.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7c964c5..4f333bd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1646,6 +1646,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long 
newmem,
 if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 isActive = virDomainObjIsActive(vm);
 
 if (flags == VIR_DOMAIN_MEM_CURRENT) {
@@ -1664,19 +1667,19 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned 
long newmem,
 if (!isActive  (flags  VIR_DOMAIN_MEM_LIVE)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(cannot set memory on an inactive domain));
-goto cleanup;
+goto endjob;
 }
 
 if (flags  VIR_DOMAIN_MEM_CONFIG) {
 if (!vm-persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(cannot change persistent config of a transient 
domain));
-goto cleanup;
+goto endjob;
 }
 if (!(persistentDef = virDomainObjGetPersistentDef(cfg-caps,
driver-xmlopt,
vm)))
-goto cleanup;
+goto endjob;
 }
 
 if (flags  VIR_DOMAIN_MEM_MAXIMUM) {
@@ -1688,7 +1691,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long 
newmem,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to set maximum memory for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -1699,7 +1702,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long 
newmem,
 if (persistentDef-mem.cur_balloon  newmem)
 persistentDef-mem.cur_balloon = newmem;
 ret = virDomainSaveConfig(cfg-configDir, persistentDef);
-goto cleanup;
+goto endjob;
 }
 
 } else {
@@ -1708,17 +1711,23 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned 
long newmem,
 if (newmem  vm-def-mem.max_balloon) {
 virReportError(VIR_ERR_INVALID_ARG, %s,
_(cannot set memory higher than max memory));
-goto cleanup;
+goto endjob;
 }
 
 if (flags  VIR_DOMAIN_MEM_LIVE) {
+int res;
+
 priv = vm-privateData;
-if (libxl_set_memory_target(priv-ctx, dom-id, newmem, 0,
-/* force */ 1)  0) {
+/* Unlock virDomainObj while ballooning memory */
+virObjectUnlock(vm);
+res = libxl_set_memory_target(priv-ctx, dom-id, newmem, 0,
+  /* force */ 1);
+virObjectLock(vm);
+if (res  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to set memory for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -1726,12 +1735,15 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned 
long newmem,
 sa_assert(persistentDef);
 persistentDef-mem.cur_balloon = newmem;
 ret = virDomainSaveConfig(cfg-configDir, persistentDef);
-goto cleanup;
+goto endjob;
 }
 }
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 if (vm)
 virObjectUnlock(vm);
-- 
1.8.1.4

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

[libvirt] [PATCH 10/11] libxl: use job functions in libxlDomainSetAutostart

[Jim Fehlig]

Setting autostart is a modify operation that needs to wait in the
queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 528c2cb..79d64a5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3855,40 +3855,43 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
 if (virDomainSetAutostartEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!vm-persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(cannot set autostart for transient domain));
-goto cleanup;
+goto endjob;
 }
 
 autostart = (autostart != 0);
 
 if (vm-autostart != autostart) {
 if (!(configFile = virDomainConfigFile(cfg-configDir, vm-def-name)))
-goto cleanup;
+goto endjob;
 if (!(autostartLink = virDomainConfigFile(cfg-autostartDir, 
vm-def-name)))
-goto cleanup;
+goto endjob;
 
 if (autostart) {
 if (virFileMakePath(cfg-autostartDir)  0) {
 virReportSystemError(errno,
  _(cannot create autostart directory %s),
  cfg-autostartDir);
-goto cleanup;
+goto endjob;
 }
 
 if (symlink(configFile, autostartLink)  0) {
 virReportSystemError(errno,
  _(Failed to create symlink '%s to '%s'),
  autostartLink, configFile);
-goto cleanup;
+goto endjob;
 }
 } else {
 if (unlink(autostartLink)  0  errno != ENOENT  errno != 
ENOTDIR) {
 virReportSystemError(errno,
  _(Failed to delete symlink '%s'),
  autostartLink);
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -3896,6 +3899,9 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
 }
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 VIR_FREE(configFile);
 VIR_FREE(autostartLink);
-- 
1.8.1.4

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

[libvirt] [PATCH 05/11] libxl: use job functions in libxlDomainDestroyFlags

[Jim Fehlig]

Modify operation that needs to wait in the queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index caabb44..bb574bc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1549,6 +1549,7 @@ libxlDomainDestroyFlags(virDomainPtr dom,
 libxlDriverPrivatePtr driver = dom-conn-privateData;
 virDomainObjPtr vm;
 int ret = -1;
+bool remove_dom = false;
 virObjectEventPtr event = NULL;
 
 virCheckFlags(0, -1);
@@ -1559,10 +1560,13 @@ libxlDomainDestroyFlags(virDomainPtr dom,
 if (virDomainDestroyFlagsEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(Domain is not running));
-goto cleanup;
+goto endjob;
 }
 
 event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
@@ -1571,17 +1575,22 @@ libxlDomainDestroyFlags(virDomainPtr dom,
 if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED) != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to destroy domain '%d'), dom-id);
-goto cleanup;
+goto endjob;
 }
 
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
+if (!vm-persistent)
+remove_dom = true;
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
+if (remove_dom) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
 if (vm)
 virObjectUnlock(vm);
 if (event)
-- 
1.8.1.4

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

[libvirt] [PATCH 11/11] libxl: use job functions in libxlDomainSetSchedulerParametersFlags

[Jim Fehlig]

Modify operation that needs to wait in the queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 79d64a5..a4b6ecd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4045,6 +4045,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
int nparams,
unsigned int flags)
 {
+libxlDriverPrivatePtr driver = dom-conn-privateData;
 libxlDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 libxl_domain_sched_params sc_info;
@@ -4067,9 +4068,12 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
 if (virDomainSetSchedulerParametersFlagsEnsureACL(dom-conn, vm-def, 
flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 
 priv = vm-privateData;
@@ -4079,14 +4083,14 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
 if (sched_id != LIBXL_SCHEDULER_CREDIT) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Only 'credit' scheduler is supported));
-goto cleanup;
+goto endjob;
 }
 
 if (libxl_domain_sched_params_get(priv-ctx, dom-id, sc_info) != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to get scheduler parameters for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 
 for (i = 0; i  nparams; ++i) {
@@ -4102,11 +4106,14 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to set scheduler parameters for domain '%d'
   with libxenlight), dom-id);
-goto cleanup;
+goto endjob;
 }
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 if (vm)
 virObjectUnlock(vm);
-- 
1.8.1.4

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

[libvirt] [PATCH 07/11] libxl: use job functions in libxlDomainCoreDump

[Jim Fehlig]

Dumping a domain's core can take considerable time.  Use the
recently added job functions and unlock the virDomainObj while
dumping core.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a804b45..84d9ca3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2059,6 +2059,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, 
unsigned int flags)
 libxlDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 virObjectEventPtr event = NULL;
+bool remove_dom = false;
 bool paused = false;
 int ret = -1;
 
@@ -2070,9 +2071,12 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, 
unsigned int flags)
 if (virDomainCoreDumpEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 
 priv = vm-privateData;
@@ -2084,38 +2088,41 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, 
unsigned int flags)
_(Before dumping core, failed to suspend domain 
'%d'
   with libxenlight),
dom-id);
-goto cleanup;
+goto endjob;
 }
 virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_DUMP);
 paused = true;
 }
 
-if (libxl_domain_core_dump(priv-ctx, dom-id, to, NULL) != 0) {
+/* Unlock virDomainObj while dumping core */
+virObjectUnlock(vm);
+ret = libxl_domain_core_dump(priv-ctx, dom-id, to, NULL);
+virObjectLock(vm);
+if (ret != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to dump core of domain '%d' with 
libxenlight),
dom-id);
-goto cleanup_unpause;
+ret = -1;
+goto unpause;
 }
 
 if (flags  VIR_DUMP_CRASH) {
 if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to destroy domain '%d'), dom-id);
-goto cleanup_unpause;
+goto unpause;
 }
 
 event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
  VIR_DOMAIN_EVENT_STOPPED_CRASHED);
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
+if (!vm-persistent)
+remove_dom = true;
 }
 
 ret = 0;
 
-cleanup_unpause:
-if (vm  virDomainObjIsActive(vm)  paused) {
+unpause:
+if (virDomainObjIsActive(vm)  paused) {
 if (libxl_domain_unpause(priv-ctx, dom-id) != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(After dumping core, failed to resume domain '%d' 
with
@@ -2125,7 +2132,15 @@ cleanup_unpause:
  VIR_DOMAIN_RUNNING_UNPAUSED);
 }
 }
+
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
+if (remove_dom) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
 if (vm)
 virObjectUnlock(vm);
 if (event)
-- 
1.8.1.4

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

[libvirt] [PATCH 09/11] libxl: use job functions in device attach and detach functions

[Jim Fehlig]

These operations aren't necessarily time consuming, but need to
wait in the queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 40 ++--
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7ce127a..528c2cb 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3414,6 +3414,9 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char 
*xml,
 if (virDomainAttachDeviceFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (virDomainObjIsActive(vm)) {
 if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
 flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
@@ -3424,14 +3427,14 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (flags  VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(Domain is not running));
-goto cleanup;
+goto endjob;
 }
 }
 
 if ((flags  VIR_DOMAIN_DEVICE_MODIFY_CONFIG)  !vm-persistent) {
  virReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(cannot modify device on transient domain));
- goto cleanup;
+ goto endjob;
 }
 
 priv = vm-privateData;
@@ -3440,15 +3443,15 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (!(dev = virDomainDeviceDefParse(xml, vm-def,
 cfg-caps, driver-xmlopt,
 VIR_DOMAIN_XML_INACTIVE)))
-goto cleanup;
+goto endjob;
 
 /* Make a copy for updated domain. */
 if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg-caps,
 driver-xmlopt)))
-goto cleanup;
+goto endjob;
 
 if ((ret = libxlDomainAttachDeviceConfig(vmdef, dev))  0)
-goto cleanup;
+goto endjob;
 } else {
 ret = 0;
 }
@@ -3459,10 +3462,10 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (!(dev = virDomainDeviceDefParse(xml, vm-def,
 cfg-caps, driver-xmlopt,
 VIR_DOMAIN_XML_INACTIVE)))
-goto cleanup;
+goto endjob;
 
 if ((ret = libxlDomainAttachDeviceLive(priv, vm, dev))  0)
-goto cleanup;
+goto endjob;
 
 /*
  * update domain status forcibly because the domain status may be
@@ -3481,6 +3484,9 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char 
*xml,
 }
 }
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 virDomainDefFree(vmdef);
 virDomainDeviceDefFree(dev);
@@ -3518,6 +3524,9 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char 
*xml,
 if (virDomainDetachDeviceFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (virDomainObjIsActive(vm)) {
 if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
 flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
@@ -3528,14 +3537,14 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (flags  VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
 virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(Domain is not running));
-goto cleanup;
+goto endjob;
 }
 }
 
 if ((flags  VIR_DOMAIN_DEVICE_MODIFY_CONFIG)  !vm-persistent) {
  virReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(cannot modify device on transient domain));
- goto cleanup;
+ goto endjob;
 }
 
 priv = vm-privateData;
@@ -3544,15 +3553,15 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (!(dev = virDomainDeviceDefParse(xml, vm-def,
 cfg-caps, driver-xmlopt,
 VIR_DOMAIN_XML_INACTIVE)))
-goto cleanup;
+goto endjob;
 
 /* Make a copy for updated domain. */
 if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg-caps,
 driver-xmlopt)))
-goto cleanup;
+goto endjob;
 
 if ((ret = libxlDomainDetachDeviceConfig(vmdef, dev))  0)
-goto cleanup;
+goto endjob;
 } else {
 ret = 0;
 }
@@ -3563,10 +3572,10 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const 
char *xml,
 if (!(dev = virDomainDeviceDefParse(xml, vm-def,
 cfg-caps, driver-xmlopt,
  

[libvirt] [PATCH 01/11] libxl: Add job support to libxl driver

[Jim Fehlig]

Follows the pattern used in the QEMU driver for managing multiple,
simultaneous jobs within the driver.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_domain.c | 128 +++
 src/libxl/libxl_domain.h |  37 ++
 2 files changed, 165 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index eb2e50e..fdc4661 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -30,10 +30,18 @@
 #include virerror.h
 #include virlog.h
 #include virstring.h
+#include virtime.h
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
 
+VIR_ENUM_IMPL(libxlDomainJob, LIBXL_JOB_LAST,
+  none,
+  query,
+  destroy,
+  modify,
+);
+
 /* Object used to store info related to libxl event registrations */
 typedef struct _libxlEventHookInfo libxlEventHookInfo;
 typedef libxlEventHookInfo *libxlEventHookInfoPtr;
@@ -284,6 +292,119 @@ static const libxl_osevent_hooks libxl_event_callbacks = {
 .timeout_deregister = libxlDomainObjTimeoutDeregisterEventHook,
 };
 
+static int
+libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv)
+{
+memset(priv-job, 0, sizeof(priv-job));
+
+if (virCondInit(priv-job.cond)  0)
+return -1;
+
+return 0;
+}
+
+static void
+libxlDomainObjResetJob(libxlDomainObjPrivatePtr priv)
+{
+struct libxlDomainJobObj *job = priv-job;
+
+job-active = LIBXL_JOB_NONE;
+job-owner = 0;
+}
+
+static void
+libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv)
+{
+ignore_value(virCondDestroy(priv-job.cond));
+}
+
+/* Give up waiting for mutex after 30 seconds */
+#define LIBXL_JOB_WAIT_TIME (1000ull * 30)
+
+/*
+ * obj must be locked before calling, libxlDriverPrivatePtr must NOT be locked
+ *
+ * This must be called by anything that will change the VM state
+ * in any way
+ *
+ * Upon successful return, the object will have its ref count increased,
+ * successful calls must be followed by EndJob eventually
+ */
+int
+libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
+   virDomainObjPtr obj,
+   enum libxlDomainJob job)
+{
+libxlDomainObjPrivatePtr priv = obj-privateData;
+unsigned long long now;
+unsigned long long then;
+
+if (virTimeMillisNow(now)  0)
+return -1;
+then = now + LIBXL_JOB_WAIT_TIME;
+
+virObjectRef(obj);
+
+while (priv-job.active) {
+VIR_DEBUG(Wait normal job condition for starting job: %s,
+  libxlDomainJobTypeToString(job));
+if (virCondWaitUntil(priv-job.cond, obj-parent.lock, then)  0)
+goto error;
+}
+
+libxlDomainObjResetJob(priv);
+
+VIR_DEBUG(Starting job: %s, libxlDomainJobTypeToString(job));
+priv-job.active = job;
+priv-job.owner = virThreadSelfID();
+
+return 0;
+
+error:
+VIR_WARN(Cannot start job (%s) for domain %s;
+  current job is (%s) owned by (%d),
+ libxlDomainJobTypeToString(job),
+ obj-def-name,
+ libxlDomainJobTypeToString(priv-job.active),
+ priv-job.owner);
+
+if (errno == ETIMEDOUT)
+virReportError(VIR_ERR_OPERATION_TIMEOUT,
+   %s, _(cannot acquire state change lock));
+else
+virReportSystemError(errno,
+ %s, _(cannot acquire job mutex));
+
+virObjectUnref(obj);
+return -1;
+}
+
+/*
+ * obj must be locked before calling
+ *
+ * To be called after completing the work associated with the
+ * earlier libxlDomainBeginJob() call
+ *
+ * Returns true if the remaining reference count on obj is
+ * non-zero, false if the reference count has dropped to zero
+ * and obj is disposed.
+ */
+bool
+libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
+ virDomainObjPtr obj)
+{
+libxlDomainObjPrivatePtr priv = obj-privateData;
+enum libxlDomainJob job = priv-job.active;
+
+VIR_DEBUG(Stopping job: %s,
+  libxlDomainJobTypeToString(job));
+
+libxlDomainObjResetJob(priv);
+virCondSignal(priv-job.cond);
+
+return virObjectUnref(obj);
+}
+
 static void *
 libxlDomainObjPrivateAlloc(void)
 {
@@ -300,6 +421,12 @@ libxlDomainObjPrivateAlloc(void)
 return NULL;
 }
 
+if (libxlDomainObjInitJob(priv)  0) {
+virChrdevFree(priv-devs);
+virObjectUnref(priv);
+return NULL;
+}
+
 return priv;
 }
 
@@ -311,6 +438,7 @@ libxlDomainObjPrivateDispose(void *obj)
 if (priv-deathW)
 libxl_evdisable_domain_death(priv-ctx, priv-deathW);
 
+libxlDomainObjFreeJob(priv);
 virChrdevFree(priv-devs);
 libxl_ctx_free(priv-ctx);
 if (priv-logger_file)
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 8565820..4decc40 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -30,6 +30,31 @@
 # include libxl_conf.h
 # include virchrdev.h
 
+# define JOB_MASK(job) 

[libvirt] [PATCH 04/11] libxl: use job functions in libxlDomain{Suspend, Resume}

[Jim Fehlig]

These operations aren't necessarily time consuming, but need to
wait in the queue of modify jobs.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4f333bd..caabb44 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1359,9 +1359,12 @@ libxlDomainSuspend(virDomainPtr dom)
 if (virDomainSuspendEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 
 priv = vm-privateData;
@@ -1371,7 +1374,7 @@ libxlDomainSuspend(virDomainPtr dom)
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to suspend domain '%d' with libxenlight),
dom-id);
-goto cleanup;
+goto endjob;
 }
 
 virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER);
@@ -1381,10 +1384,13 @@ libxlDomainSuspend(virDomainPtr dom)
 }
 
 if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
-goto cleanup;
+goto endjob;
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 if (vm)
 virObjectUnlock(vm);
@@ -1411,9 +1417,12 @@ libxlDomainResume(virDomainPtr dom)
 if (virDomainResumeEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 
 priv = vm-privateData;
@@ -1423,7 +1432,7 @@ libxlDomainResume(virDomainPtr dom)
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to resume domain '%d' with libxenlight),
dom-id);
-goto cleanup;
+goto endjob;
 }
 
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
@@ -1434,10 +1443,13 @@ libxlDomainResume(virDomainPtr dom)
 }
 
 if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm)  0)
-goto cleanup;
+goto endjob;
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
 if (vm)
 virObjectUnlock(vm);
-- 
1.8.1.4

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

[libvirt] [PATCH 06/11] libxl: use job functions in domain save operations

[Jim Fehlig]

Saving domain memory and cpu state can take considerable time.
Use the recently added job functions and unlock the virDomainObj
while saving the domain.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 54 ++--
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index bb574bc..a804b45 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1900,10 +1900,16 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 goto cleanup;
 }
 
-if (libxl_domain_suspend(priv-ctx, vm-def-id, fd, 0, NULL) != 0) {
+/* Unlock virDomainObj while saving domain */
+virObjectUnlock(vm);
+ret = libxl_domain_suspend(priv-ctx, vm-def-id, fd, 0, NULL);
+virObjectLock(vm);
+
+if (ret != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to save domain '%d' with libxenlight),
vm-def-id);
+ret = -1;
 goto cleanup;
 }
 
@@ -1935,6 +1941,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, 
const char *dxml,
 libxlDriverPrivatePtr driver = dom-conn-privateData;
 virDomainObjPtr vm;
 int ret = -1;
+bool remove_dom = false;
 
 virCheckFlags(0, -1);
 if (dxml) {
@@ -1949,22 +1956,30 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, 
const char *dxml,
 if (virDomainSaveFlagsEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 
 if (libxlDoDomainSave(driver, vm, to)  0)
-goto cleanup;
+goto endjob;
 
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
+if (!vm-persistent)
+remove_dom = true;
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
+if (remove_dom) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
 if (vm)
 virObjectUnlock(vm);
 return ret;
@@ -2125,6 +2140,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int 
flags)
 virDomainObjPtr vm = NULL;
 char *name = NULL;
 int ret = -1;
+bool remove_dom = false;
 
 virCheckFlags(0, -1);
 
@@ -2134,33 +2150,41 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int 
flags)
 if (virDomainManagedSaveEnsureACL(dom-conn, vm-def)  0)
 goto cleanup;
 
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY)  0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s, _(Domain is not 
running));
-goto cleanup;
+goto endjob;
 }
 if (!vm-persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(cannot do managed save for transient domain));
-goto cleanup;
+goto endjob;
 }
 
 name = libxlDomainManagedSavePath(driver, vm);
 if (name == NULL)
-goto cleanup;
+goto endjob;
 
 VIR_INFO(Saving state to %s, name);
 
 if (libxlDoDomainSave(driver, vm, name)  0)
-goto cleanup;
+goto endjob;
 
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
+if (!vm-persistent)
+remove_dom = true;
 
 ret = 0;
 
+endjob:
+libxlDomainObjEndJob(driver, vm);
+
 cleanup:
+if (remove_dom) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
 if (vm)
 virObjectUnlock(vm);
 VIR_FREE(name);
-- 
1.8.1.4

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

Re: [libvirt] [Qemu-devel] Looking for project ideas and mentors for Google Summer of Code 2014

[Fam Zheng]

On Thu, 02/06 13:27, Stefan Hajnoczi wrote:
 On Tue, Feb 4, 2014 at 1:38 PM, Michal Privoznik mpriv...@redhat.com wrote:
  On 03.02.2014 08:45, Stefan Hajnoczi wrote:
 
  KVM  libvirt: you are welcome to join the QEMU umbrella organization
  like last year.
 
 
  I've updated wiki with a libvirt idea. But I can sense more to come later as
  I have some time to think about it :)
 
 Great, thanks!
 
 I have added my QEMU block layer ideas too.
 
 Deadline for the organization application is 14th of February.  As
 part of the application form we need to show our list of ideas.
 Thanks for posting your project ideas so that our list is ready.
 

I'd like to add persistent dirty bitmap as an idea but I seem to have no
account on wiki, so I'll just reply here, please help with review and update
the page if it makes sense. (Who could create an account for me, BTW?)

Project Idea Below
==

Incremental backup of block images
--

Summary: Implement persistent incremental image backup.

Users want to do regular backup of VM image data to protect data from
unexpected loss.  Incremental backup is a backup strategy that only copies out
the new data that is changed since previous backup, to reduce the overhead of
backup and improve the storage utilization. To track which part of guest data
is changed, QEMU needs to store image's dirty bitmap on the disk as well as
the image data itself.

The task is to implement a new block driver (a filter) to load/store this
persistent dirty bitmap file, and maintain the dirty bits while the guest
writes to the data image. As a prerequisite, you also need to make the design
of this bitmap file format. Then, design test cases and write scripts to test
the driver.

The persistent bitmap file must contain:

0. Magic bits to identify the format of this file.
1. Bitmap granularity (e.g. 64 KB)
2. The actual bitmap (1 TB disk @ 64 KB granularity = 2 MB bitmap)
3. Flags including a clean flag. The clean flag is used to tell whether the
   persistent bitmap file is safe to use again.  When QEMU opens the persistent
   dirty bitmap, it clears the clean flag.  When QEMU deactivates and
   finishes writing out the dirty bitmap, it sets the clean flag.  If the
   QEMU process crashes it is not safe to trust the dirty bitmap; a full backup
   must be performed. Make use of this flag in the driver to limit the
   performance overhead.

Links:
[http://en.wikipedia.org/wiki/Incremental_backup Incremental backup]
[http://lists.nongnu.org/archive/html/qemu-devel/2014-01/msg02156.html QMP:
Introduce incremental drive-backup with in-memory dirty bitmap]

Details:
Skill level: intermediate
Language: C
Mentors:  Fam Zheng f...@redhat.com (fam on IRC),
  Stefan Hajnoczi stefa...@redhat.com (stefanha on IRC)

Thanks,
Fam

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

[libvirt] [PATCH v5] bhyve: add a basic driver

[Roman Bogorodskiy]

Changes from v4:
 - Set acpi and apic flags based on domain definition
 - Add more detailed description about -H and -P flags
   of bhyve to justify theirs usage

Roman Bogorodskiy (1):
  bhyve: add a basic driver

 configure.ac|   7 +
 daemon/libvirtd.c   |   9 +
 include/libvirt/virterror.h |   1 +
 m4/virt-driver-bhyve.m4 |  57 +
 po/POTFILES.in  |   3 +
 src/Makefile.am |  30 +++
 src/bhyve/bhyve_command.c   | 279 ++
 src/bhyve/bhyve_command.h   |  41 
 src/bhyve/bhyve_driver.c| 566 
 src/bhyve/bhyve_driver.h|  28 +++
 src/bhyve/bhyve_process.c   | 216 +
 src/bhyve/bhyve_process.h   |  36 +++
 src/bhyve/bhyve_utils.h |  48 
 src/conf/domain_conf.c  |   3 +-
 src/conf/domain_conf.h  |   1 +
 src/driver.h|   1 +
 src/libvirt.c   |   3 +
 src/util/virerror.c |   1 +
 18 files changed, 1329 insertions(+), 1 deletion(-)
 create mode 100644 m4/virt-driver-bhyve.m4
 create mode 100644 src/bhyve/bhyve_command.c
 create mode 100644 src/bhyve/bhyve_command.h
 create mode 100644 src/bhyve/bhyve_driver.c
 create mode 100644 src/bhyve/bhyve_driver.h
 create mode 100644 src/bhyve/bhyve_process.c
 create mode 100644 src/bhyve/bhyve_process.h
 create mode 100644 src/bhyve/bhyve_utils.h

-- 
1.8.4.3

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

[libvirt] [PATCH v5] bhyve: add a basic driver

[Roman Bogorodskiy]

At this point it has a limited functionality and is highly
experimental. Supported domain operations are:

  * define
  * start
  * destroy
  * dumpxml
  * dominfo

It's only possible to have only one disk device and only one
network, which should be of type bridge.
---
 configure.ac|   7 +
 daemon/libvirtd.c   |   9 +
 include/libvirt/virterror.h |   1 +
 m4/virt-driver-bhyve.m4 |  57 +
 po/POTFILES.in  |   3 +
 src/Makefile.am |  30 +++
 src/bhyve/bhyve_command.c   | 279 ++
 src/bhyve/bhyve_command.h   |  41 
 src/bhyve/bhyve_driver.c| 566 
 src/bhyve/bhyve_driver.h|  28 +++
 src/bhyve/bhyve_process.c   | 216 +
 src/bhyve/bhyve_process.h   |  36 +++
 src/bhyve/bhyve_utils.h |  48 
 src/conf/domain_conf.c  |   3 +-
 src/conf/domain_conf.h  |   1 +
 src/driver.h|   1 +
 src/libvirt.c   |   3 +
 src/util/virerror.c |   1 +
 18 files changed, 1329 insertions(+), 1 deletion(-)
 create mode 100644 m4/virt-driver-bhyve.m4
 create mode 100644 src/bhyve/bhyve_command.c
 create mode 100644 src/bhyve/bhyve_command.h
 create mode 100644 src/bhyve/bhyve_driver.c
 create mode 100644 src/bhyve/bhyve_driver.h
 create mode 100644 src/bhyve/bhyve_process.c
 create mode 100644 src/bhyve/bhyve_process.h
 create mode 100644 src/bhyve/bhyve_utils.h

diff --git a/configure.ac b/configure.ac
index 23e2201..8b88f21 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1048,6 +1048,12 @@ fi
 AM_CONDITIONAL([WITH_PARALLELS], [test $with_parallels = yes])
 
 dnl
+dnl Checks for bhyve driver
+dnl
+
+LIBVIRT_DRIVER_CHECK_BHYVE
+
+dnl
 dnl check for shell that understands  redirection without truncation,
 dnl needed by src/qemu/qemu_monitor_{text,json}.c.
 dnl
@@ -2704,6 +2710,7 @@ AC_MSG_NOTICE([ PHYP: $with_phyp])
 AC_MSG_NOTICE([  ESX: $with_esx])
 AC_MSG_NOTICE([  Hyper-V: $with_hyperv])
 AC_MSG_NOTICE([Parallels: $with_parallels])
+LIBIVRT_DRIVER_RESULT_BHYVE
 AC_MSG_NOTICE([ Test: $with_test])
 AC_MSG_NOTICE([   Remote: $with_remote])
 AC_MSG_NOTICE([  Network: $with_network])
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 49c42ad..b27c6fd 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -77,6 +77,9 @@
 # ifdef WITH_VBOX
 #  include vbox/vbox_driver.h
 # endif
+# ifdef WITH_BHYVE
+#  include bhyve/bhyve_driver.h
+# endif
 # ifdef WITH_NETWORK
 #  include network/bridge_driver.h
 # endif
@@ -405,6 +408,9 @@ static void daemonInitialize(void)
 # ifdef WITH_VBOX
 virDriverLoadModule(vbox);
 # endif
+# ifdef WITH_BHYVE
+virDriverLoadModule(bhyve);
+# endif
 #else
 # ifdef WITH_NETWORK
 networkRegister();
@@ -442,6 +448,9 @@ static void daemonInitialize(void)
 # ifdef WITH_VBOX
 vboxRegister();
 # endif
+# ifdef WITH_BHYVE
+bhyveRegister();
+# endif
 #endif
 }
 
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index e31e9c4..7915bbb 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -121,6 +121,7 @@ typedef enum {
 VIR_FROM_ACCESS = 55,   /* Error from access control manager */
 VIR_FROM_SYSTEMD = 56,  /* Error from systemd code */
 
+VIR_FROM_BHYVE = 57,/* Error from bhyve driver */
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
 # endif
diff --git a/m4/virt-driver-bhyve.m4 b/m4/virt-driver-bhyve.m4
new file mode 100644
index 000..969133a
--- /dev/null
+++ b/m4/virt-driver-bhyve.m4
@@ -0,0 +1,57 @@
+dnl The bhyve driver
+dnl
+dnl Copyright (C) 2014 Roman Bogorodskiy
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl http://www.gnu.org/licenses/.
+dnl
+
+AC_DEFUN([LIBVIRT_DRIVER_CHECK_BHYVE],[
+AC_ARG_WITH([bhyve],
+  [AS_HELP_STRING([--with-bhyve],
+[add BHyVe support @:@default=check@:@])])
+m4_divert_text([DEFAULTS], [with_bhyve=check])
+
+if test $with_bhyve != no; then
+AC_PATH_PROG([BHYVE], [bhyve], [], [$PATH:/usr/sbin])
+AC_PATH_PROG([BHYVECTL], [bhyvectl], [$PATH:/usr/sbin])
+AC_PATH_PROG([BHYVELOAD], [bhyveload], [$PATH:/usr/sbin/])
+
+if test -z $BHYVE || test -z $BHYVECTL \
+test -z $BHYVELOAD || test $with_freebsd = no; then
+if test $with_bhyve = check; then
+with_bhyve=no
+else