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

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

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Chen Hanxiao
> Sent: Monday, July 14, 2014 6:02 PM
> To: libvir-list@redhat.com
> Subject: [libvirt] [PATCH] LXC: create a bind mount for sysfs when enable 
> userns
> but disable netns
> 
> kernel commit 7dc5dbc879bd0779924b5132a48b731a0bc04a1e
> forbid us doing a fresh mount for sysfs
> when enable userns but disable netns.
> This patch will create a bind mount in this senario.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  src/lxc/lxc_container.c | 44 +---
>  1 file changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 4d89677..8a27215 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -815,10 +815,13 @@ static int lxcContainerSetReadOnly(void)
>  }
> 
> 
> -static int lxcContainerMountBasicFS(bool userns_enabled)
> +static int lxcContainerMountBasicFS(bool userns_enabled,
> +bool netns_disabled)
>  {
>  size_t i;
>  int rc = -1;
> +char* mnt_src = NULL;
> +int mnt_mflags;
> 
>  VIR_DEBUG("Mounting basic filesystems");
> 
> @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
>  bool bindOverReadonly;
>  virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
> 
> +/* When enable userns but disable netns, kernel will
> + * forbid us doing a new fresh mount for sysfs.
> + * So we had to do a bind mount for sysfs instead.
> + */
> +if (userns_enabled && netns_disabled &&
> +STREQ(mnt->src, "sysfs")) {
> +if (VIR_STRDUP(mnt_src, "/sys") < 0) {
> +goto cleanup;
> +}
> +mnt_mflags = MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY|MS_BIND;
> +} else {
> +if (VIR_STRDUP(mnt_src, mnt->src) < 0) {
> +goto cleanup;
> +}
> +mnt_mflags = mnt->mflags;
> +}
> +
>  VIR_DEBUG("Processing %s -> %s",
> -  mnt->src, mnt->dst);
> +  mnt_src, mnt->dst);
> 
>  if (mnt->skipUnmounted) {
>  char *hostdir;
> @@ -856,7 +876,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
>  if (virFileMakePath(mnt->dst) < 0) {
>  virReportSystemError(errno,
>   _("Failed to mkdir %s"),
> - mnt->src);
> + mnt_src);
>  goto cleanup;
>  }
> 
> @@ -867,24 +887,24 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
>   * we mount the filesystem in read-write mode initially, and then do 
> a
>   * separate read-only bind mount on top of that.
>   */
> -bindOverReadonly = !!(mnt->mflags & MS_RDONLY);
> +bindOverReadonly = !!(mnt_mflags & MS_RDONLY);
> 
>  VIR_DEBUG("Mount %s on %s type=%s flags=%x",
> -  mnt->src, mnt->dst, mnt->type, mnt->mflags & ~MS_RDONLY);
> -if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags & ~MS_RDONLY, 
> NULL)
> < 0) {
> +  mnt_src, mnt->dst, mnt->type, mnt_mflags & ~MS_RDONLY);
> +if (mount(mnt_src, mnt->dst, mnt->type, mnt_mflags & ~MS_RDONLY, 
> NULL)
> < 0) {
>  virReportSystemError(errno,
>   _("Failed to mount %s on %s type %s 
> flags=%x"),
> - mnt->src, mnt->dst, NULLSTR(mnt->type),
> - mnt->mflags & ~MS_RDONLY);
> + mnt_src, mnt->dst, NULLSTR(mnt->type),
> + mnt_mflags & ~MS_RDONLY);
>  goto cleanup;
>  }
> 
>  if (bindOverReadonly &&
> -mount(mnt->src, mnt->dst, NULL,
> +mount(mnt_src, mnt->dst, NULL,
>MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
>  virReportSystemError(errno,
>   _("Failed to re-mount %s on %s flags=%x"),
> - mnt->src, mnt->dst,
> + mnt_src, mnt->dst,
>   MS_BIND|MS_REMOUNT|MS_RDONLY);
>  goto cleanup;
>  }
> @@ -893,6 +913,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
>  rc = 0;
> 
>   cleanup:
> +VIR_FREE(mnt_src);
>  VIR_DEBUG("rc=%d", rc);
>  return rc;
>  }
> @@ -1643,7 +1664,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr
> vmDef,
>  goto cleanup;
> 
>  /* Mounts the core /proc, /sys, etc filesystems */
> -if (lxcContainerMountBasicFS(vmDef->idmap.nuidmap) < 0)
> +if (lxcContainerMountBasicFS(vmDef->idmap.nuidmap,
> + !vmDef->nnets) < 0)
>  goto cleanup;
> 
>  /* Ensure entire roo

[libvirt] [PATCH 2/2] remove error message when virMutexInit and virCondInit failed

2014-07-17 Thread Jincheng Miao
Signed-off-by: Jincheng Miao 
---
 daemon/remote.c |  1 -
 src/conf/interface_conf.c   |  2 --
 src/conf/network_conf.c |  2 --
 src/conf/node_device_conf.c |  2 --
 src/conf/nwfilter_conf.c|  2 --
 src/conf/object_event.c |  2 --
 src/conf/storage_conf.c |  2 --
 src/conf/virchrdev.c|  2 --
 src/esx/esx_vi.c| 15 +++
 src/fdstream.c  |  2 --
 src/libxl/libxl_driver.c|  2 --
 src/locking/lock_daemon.c   |  5 -
 src/node_device/node_device_udev.c  |  1 -
 src/nwfilter/nwfilter_learnipaddr.c |  2 --
 src/parallels/parallels_driver.c|  5 +
 src/qemu/qemu_agent.c   |  2 --
 src/qemu/qemu_capabilities.c|  2 --
 src/qemu/qemu_driver.c  |  1 -
 src/qemu/qemu_monitor.c |  6 ++
 src/remote/remote_driver.c  |  2 --
 src/rpc/virnetclient.c  |  5 +
 src/test/test_driver.c  |  4 
 src/util/vireventpoll.c |  5 +
 src/util/virlockspace.c |  4 
 src/util/virobject.c|  2 --
 src/xen/xen_driver.c|  2 --
 tests/commandtest.c |  4 +---
 tests/qemumonitortestutils.c|  2 --
 28 files changed, 9 insertions(+), 79 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index ea16789..d3cf8a8 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1164,7 +1164,6 @@ void *remoteClientInitHook(virNetServerClientPtr client,
 
 if (virMutexInit(&priv->lock) < 0) {
 VIR_FREE(priv);
-virReportSystemError(errno, "%s", _("unable to init mutex"));
 return NULL;
 }
 
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 103e878..87dc79e 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -1250,8 +1250,6 @@ virInterfaceObjPtr 
virInterfaceAssignDef(virInterfaceObjListPtr interfaces,
 if (VIR_ALLOC(iface) < 0)
 return NULL;
 if (virMutexInit(&iface->lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
 VIR_FREE(iface);
 return NULL;
 }
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ce4d4d8..574727a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -388,8 +388,6 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
 if (VIR_ALLOC(network) < 0)
 return NULL;
 if (virMutexInit(&network->lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
 VIR_FREE(network);
 return NULL;
 }
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 30aa477..ed4cd2a 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -183,8 +183,6 @@ virNodeDeviceObjPtr 
virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
 return NULL;
 
 if (virMutexInit(&device->lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
 VIR_FREE(device);
 return NULL;
 }
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 52f24e4..b662e02 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -3104,8 +3104,6 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
 return NULL;
 
 if (virMutexInitRecursive(&nwfilter->lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("cannot initialize mutex"));
 VIR_FREE(nwfilter);
 return NULL;
 }
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index beef3e1..86d0eb8 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -577,8 +577,6 @@ virObjectEventStateNew(void)
 goto error;
 
 if (virMutexInit(&state->lock) < 0) {
-virReportSystemError(errno, "%s",
- _("unable to initialize state mutex"));
 VIR_FREE(state);
 goto error;
 }
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 69333f5..1e8edff 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1694,8 +1694,6 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
 return NULL;
 
 if (virMutexInit(&pool->lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("cannot initialize mutex"));
 VIR_FREE(pool);
 return NULL;
 }
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 022fe71..a659b62 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -272,8 +272,6 @@ virChrdevsPtr virChrdevAlloc(void)
 return NULL;
 
 if (virMutexInit(&devs->lock) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to init device stream 

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

2014-07-17 Thread Jincheng Miao
Implement InitInternal functions for Mutex, RWLock and Cond, these functions
contain error report using virReportSystemErrorFull, it is controlled by
an argument 'quite'.
The related macros are Init and InitQuite, they call InitInternal function
by passing 'false' or 'true' to quite argument.

Signed-off-by: Jincheng Miao 
---
 src/libvirt_private.syms |  7 ++--
 src/util/virthread.c | 95 
 src/util/virthread.h | 57 ++---
 3 files changed, 94 insertions(+), 65 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d3671c..b1c05d3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2015,18 +2015,17 @@ virSystemdTerminateMachine;
 # util/virthread.h
 virCondBroadcast;
 virCondDestroy;
-virCondInit;
+virCondInitInternal;
 virCondSignal;
 virCondWait;
 virCondWaitUntil;
 virMutexDestroy;
-virMutexInit;
-virMutexInitRecursive;
+virMutexInitInternal;
 virMutexLock;
 virMutexUnlock;
 virOnce;
 virRWLockDestroy;
-virRWLockInit;
+virRWLockInitInternal;
 virRWLockRead;
 virRWLockUnlock;
 virRWLockWrite;
diff --git a/src/util/virthread.c b/src/util/virthread.c
index 7e841d1..ceed1bf 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -31,6 +31,16 @@
 
 #include "viralloc.h"
 
+#define VIR_THREAD_ERR_EXIT(str) do {   \
+errno = ret;\
+if (!quite) {   \
+virReportSystemErrorFull(VIR_FROM_NONE, errno,  \
+ filename, funcname, linenr,\
+ "%s", _(str)); \
+}   \
+return -1;  \
+} while (0)
+
 
 /* Nothing special required for pthreads */
 int virThreadInitialize(void)
@@ -48,92 +58,81 @@ int virOnce(virOnceControlPtr once, virOnceFunc init)
 }
 
 
-int virMutexInit(virMutexPtr m)
+int virMutexInitInternal(virMutexPtr mutex, bool recursive, bool quite,
+ const char *filename, const char *funcname,
+ size_t linenr)
 {
 int ret;
 pthread_mutexattr_t attr;
+int type = recursive ? PTHREAD_MUTEX_RECURSIVE : PTHREAD_MUTEX_NORMAL;
 pthread_mutexattr_init(&attr);
-pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL);
-ret = pthread_mutex_init(&m->lock, &attr);
+pthread_mutexattr_settype(&attr, type);
+ret = pthread_mutex_init(&mutex->lock, &attr);
 pthread_mutexattr_destroy(&attr);
-if (ret != 0) {
-errno = ret;
-return -1;
-}
-return 0;
-}
 
-int virMutexInitRecursive(virMutexPtr m)
-{
-int ret;
-pthread_mutexattr_t attr;
-pthread_mutexattr_init(&attr);
-pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
-ret = pthread_mutex_init(&m->lock, &attr);
-pthread_mutexattr_destroy(&attr);
-if (ret != 0) {
-errno = ret;
-return -1;
-}
+if (ret != 0)
+VIR_THREAD_ERR_EXIT("unable to init Mutex");
+
 return 0;
 }
 
-void virMutexDestroy(virMutexPtr m)
+void virMutexDestroy(virMutexPtr mutex)
 {
-pthread_mutex_destroy(&m->lock);
+pthread_mutex_destroy(&mutex->lock);
 }
 
-void virMutexLock(virMutexPtr m)
+void virMutexLock(virMutexPtr mutex)
 {
-pthread_mutex_lock(&m->lock);
+pthread_mutex_lock(&mutex->lock);
 }
 
-void virMutexUnlock(virMutexPtr m)
+void virMutexUnlock(virMutexPtr mutex)
 {
-pthread_mutex_unlock(&m->lock);
+pthread_mutex_unlock(&mutex->lock);
 }
 
 
-int virRWLockInit(virRWLockPtr m)
+int virRWLockInitInternal(virRWLockPtr rwlock, bool quite,
+  const char *filename, const char *funcname,
+  size_t linenr)
 {
-int ret;
-ret = pthread_rwlock_init(&m->lock, NULL);
-if (ret != 0) {
-errno = ret;
-return -1;
-}
+int ret = pthread_rwlock_init(&rwlock->lock, NULL);
+if (ret != 0)
+VIR_THREAD_ERR_EXIT("unable to init RWLock");
+
 return 0;
 }
 
-void virRWLockDestroy(virRWLockPtr m)
+void virRWLockDestroy(virRWLockPtr rwlock)
 {
-pthread_rwlock_destroy(&m->lock);
+pthread_rwlock_destroy(&rwlock->lock);
 }
 
 
-void virRWLockRead(virRWLockPtr m)
+void virRWLockRead(virRWLockPtr rwlock)
 {
-pthread_rwlock_rdlock(&m->lock);
+pthread_rwlock_rdlock(&rwlock->lock);
 }
 
-void virRWLockWrite(virRWLockPtr m)
+void virRWLockWrite(virRWLockPtr rwlock)
 {
-pthread_rwlock_wrlock(&m->lock);
+pthread_rwlock_wrlock(&rwlock->lock);
 }
 
 
-void virRWLockUnlock(virRWLockPtr m)
+void virRWLockUnlock(virRWLockPtr rwlock)
 {
-pthread_rwlock_unlock(&m->lock);
+pthread_rwlock_unlock(&rwlock->lock);
 }
 
-int virCondInit(virCondPtr c)
+int virCondInitInternal(virCondPtr cond, bool

[libvirt] [PATCH 0/2] Refactor virMutexInit virRWLockInit and virCondInit

2014-07-17 Thread Jincheng Miao
Implement virMutexInitInternal, virRWLockInitInternal and virCondInitInternal
which include error message reporting.

int virMutexInitInternal(virMutexPtr mutex, bool recursive, bool quite,
 const char *filename, const char *funcname,
 size_t linenr)

virMutexInit calls virMutexInitInternal with quite == false;
virMutexInitQuite calls virMutexInitInternal with quite == true.
Same for virRWLockInit and virCondInit.

And remove redundant error messages after virMutexInit from other source files.

Jincheng Miao (2):
  Refactor virMutexInit virRWLockInit and virCondInit
  remove error message when virMutexInit and virCondInit failed

 daemon/remote.c |  1 -
 src/conf/interface_conf.c   |  2 -
 src/conf/network_conf.c |  2 -
 src/conf/node_device_conf.c |  2 -
 src/conf/nwfilter_conf.c|  2 -
 src/conf/object_event.c |  2 -
 src/conf/storage_conf.c |  2 -
 src/conf/virchrdev.c|  2 -
 src/esx/esx_vi.c| 15 ++
 src/fdstream.c  |  2 -
 src/libvirt_private.syms|  7 ++-
 src/libxl/libxl_driver.c|  2 -
 src/locking/lock_daemon.c   |  5 --
 src/node_device/node_device_udev.c  |  1 -
 src/nwfilter/nwfilter_learnipaddr.c |  2 -
 src/parallels/parallels_driver.c|  5 +-
 src/qemu/qemu_agent.c   |  2 -
 src/qemu/qemu_capabilities.c|  2 -
 src/qemu/qemu_driver.c  |  1 -
 src/qemu/qemu_monitor.c |  6 +--
 src/remote/remote_driver.c  |  2 -
 src/rpc/virnetclient.c  |  5 +-
 src/test/test_driver.c  |  4 --
 src/util/vireventpoll.c |  5 +-
 src/util/virlockspace.c |  4 --
 src/util/virobject.c|  2 -
 src/util/virthread.c| 95 ++---
 src/util/virthread.h| 57 +-
 src/xen/xen_driver.c|  2 -
 tests/commandtest.c |  4 +-
 tests/qemumonitortestutils.c|  2 -
 31 files changed, 103 insertions(+), 144 deletions(-)

-- 
1.8.3.1

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


Re: [libvirt] CPU model API (v3)

2014-07-17 Thread Zeeshan Ali (Khattak)
On Tue, Jul 15, 2014 at 11:42 PM, Zeeshan Ali (Khattak)
 wrote:
> v3: Two classes for CPU model:
>   * CapabilitiesCpuModel
>   * DomainCpuModel that derives from CapabilitiesCpuModel.

Ping! I need these patches to fix the broken case of non-kvm in Boxes
in a nice way:

https://bugzilla.gnome.org/show_bug.cgi?id=732680

-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/

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


Re: [libvirt] [PATCH] libxl: Implement basic video device selection

2014-07-17 Thread Jim Fehlig
Stefan Bader wrote:
> On 16.07.2014 23:05, Jim Fehlig wrote:
>   
>> While testing this, I noticed that libvirt will set vram to 9216 if not
>> specified.  E.g.
>>
>> # cat test.xml
>> ...
>> 
>>   
>> 
>> ...
>> # virsh define test.xml
>> # virsh dumpxml test
>> ...
>> 
>>   
>> 
>> ...
>>
>> With type='vga', libxl will then fail to start the domain
>>
>> libxl: error: libxl_create.c:253:libxl__domain_build_info_setdefault:
>> videoram must be at least 16 MB for STDVGA on QEMU_XEN
>> 
>
> Heh, thats "funny". At least this was the same thing I observed when creating
> new guests with virt-manager. Maybe I blamed the wrong part of the stack. Or
> they both do it. This lead the the second part of my changes which was not so
> clear about whether it should or should not go upstream.
>
> So from my side a fixup of some kind would be good but we can work on this in 
> a
> follow-up.
>   

The problem is, some domains may not start after applying this patch. 
Consider a domain with the following video config


  


This would work pre-patch since libxl_domain_build_info->video_memkb
would be initialized to LIBXL_MEMKB_DEFAULT when calling
libxl_domain_build_info_init().  But the domain will fail to start
post-patch since video_memkb is set to an invalid value.

I know the current behavior is not correct either, but would be nice to
fix everything up in one series.  I started on a second patch that would
validate input or set sane defaults in libxlDomainDeviceDefPostParse(),
but while testing realized that sane defaults depend on which qemu is
used.  E.g. the minimum video_memkb values have doubled with QEMU_XEN vs
QEMU_XEN_TRADITIONAL - see $xen_root/tools/libxl/libxl_create.c,
libxl__domain_build_info_setdefault().  [I'll mention again that it is
unfortunate that QEMU_XEN vs QEMU_XEN_TRADITIONAL leaked through the
public API.]

I'm thinking of writing a utility function to detect the old vs new
qemu, hoping there is something in the help output or similar to
determine which one  is.  Such a function would be useful for
David Scott's old patch to support arbitrary user-provided 

https://www.redhat.com/archives/libvir-list/2013-April/msg02119.html

But alas, this will have to wait until I return from vacation.

Regards,
Jim


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


Re: [libvirt] [PATCH 0/2] Disallow wiping a sparse logical volume

2014-07-17 Thread John Ferlan


On 07/17/2014 02:10 PM, John Ferlan wrote:

> John Ferlan (2):
>   storage: Convert 'building' into a bool
>   storage: Disallow vol_wipe for sparse logical volumes
> 
>  src/conf/storage_conf.h   |  2 +-
>  src/storage/storage_backend_logical.c | 39 
> ++-
>  src/storage/storage_driver.c  |  8 +++
>  src/util/virstoragefile.h |  1 +
>  4 files changed, 44 insertions(+), 6 deletions(-)
> 

Thanks for the review - I adjusted the completely... funny when I typed
it and looked at it I thought it looked funny but my fingers were more
insistent that they had done the right thing :-)

Not sure lvremove/lvresize will do the trick here - read the 'lvmthin'
man page...  Also online I found here was a reading or two online that
suggested one cannot reduce the size of a thin pool due to some lvm bug.
There's another doc that says in order to reduce the size use trim, but
I'm not quite sure how to access that.

In my "example" case - my extent size is 4096 and my thin/sparse size is
4096.  Attemping to lvreduce results in a "New size given (xxx) not less
than existing size (1 extents)."  So even though I've filled half of it
with 0's, there doesn't seem to be a way to do what we want.

Maybe someone has more "experience" dealing with these

In any case, I pushed what I have...

John

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


Re: [libvirt] [PATCHv2 1/2] Introduce virTristateBool enum type

2014-07-17 Thread John Ferlan


On 07/16/2014 04:34 AM, Ján Tomko wrote:
> Replace all three-state (default/yes/no) enums with it:
> virDomainBootMenu
> virDomainPMState
> virDomainGraphicsSpiceClipboardCopypaste
> virDomainGraphicsSpiceAgentFileTransfer
> virNetworkDNSForwardPlainNames

virDomainBIOSUseserial

Too.


> ---
>  src/conf/domain_conf.c  | 57 
> -
>  src/conf/domain_conf.h  | 43 --
>  src/conf/network_conf.c | 11 ++---
>  src/conf/network_conf.h | 15 +---
>  src/libvirt_private.syms| 10 ++--
>  src/network/bridge_driver.c |  3 +--
>  src/qemu/qemu_command.c | 22 -
>  src/qemu/qemu_driver.c  |  4 ++--
>  src/util/virutil.c  |  5 
>  src/util/virutil.h  | 11 +
>  10 files changed, 50 insertions(+), 131 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1d83f13..e374604 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -134,11 +134,6 @@ VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST,
>"hd",
>"network")
>  
> -VIR_ENUM_IMPL(virDomainBootMenu, VIR_DOMAIN_BOOT_MENU_LAST,
> -  "default",
> -  "yes",
> -  "no")
> -
>  VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
>"acpi",
>"apic",
> @@ -180,11 +175,6 @@ VIR_ENUM_IMPL(virDomainLockFailure, 
> VIR_DOMAIN_LOCK_FAILURE_LAST,
>"pause",
>"ignore")
>  
> -VIR_ENUM_IMPL(virDomainPMState, VIR_DOMAIN_PM_STATE_LAST,
> -  "default",
> -  "yes",
> -  "no")
> -
>  VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
>"none",
>"disk",
> @@ -576,18 +566,6 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode,
>"all",
>"off");
>  
> -VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
> -  VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST,
> -  "default",
> -  "yes",
> -  "no");
> -
> -VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer,
> -  VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST,
> -  "default",
> -  "yes",
> -  "no");
> -
>  VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
>"subsystem",
>"capabilities")
> @@ -8789,7 +8767,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
>  }
>  
>  if ((copypasteVal =
> - 
> virDomainGraphicsSpiceClipboardCopypasteTypeFromString(copypaste)) <= 0) {
> + virTristateBoolTypeFromString(copypaste)) <= 0) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown copypaste value '%s'"), 
> copypaste);
>  VIR_FREE(copypaste);
> @@ -8809,7 +8787,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
>  }
>  
>  if ((enableVal =
> - 
> virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) {
> + virTristateBoolTypeFromString(enable)) <= 0) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown enable value '%s'"), 
> enable);
>  VIR_FREE(enable);
> @@ -9916,7 +9894,7 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,
>  int ret = -1;
>  char *tmp = virXPathString(xpath, ctxt);
>  if (tmp) {
> -*val = virDomainPMStateTypeFromString(tmp);
> +*val = virTristateBoolTypeFromString(tmp);
>  if (*val < 0) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown PM state value %s"), tmp);
> @@ -10879,14 +10857,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>  
>  tmp = virXPathString("string(./os/bootmenu[1]/@enable)", ctxt);
>  if (tmp) {
> -def->os.bootmenu = virDomainBootMenuTypeFromString(tmp);
> +def->os.bootmenu = virTristateBoolTypeFromString(tmp);
>  if (def->os.bootmenu <= 0) {
>  /* In order not to break misconfigured machines, this
>   * should not emit an error, but rather set the bootmenu
>   * to disabled */
>  VIR_WARN("disabling bootmenu due to unknown option '%s'",
>   tmp);
> -def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED;
> +def->os.bootmenu = VIR_TRISTATE_BOOL_NO;
>  }
>  VIR_FREE(tmp);
>  }
> @@ -10901,9 +10879,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>   "for useserial"));
>  goto cleanup;
>  }
> -def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES

Re: [libvirt] [PATCH] examples: Introduce domtop

2014-07-17 Thread Eric Blake
On 07/16/2014 08:27 AM, Eric Blake wrote:
> On 07/16/2014 07:53 AM, Michal Privoznik wrote:
>> There's this question on the list that is asked over and over again.
>> How do I get {cpu, memory, ...} usage in percentage? Or its modified
>> version: How do I plot nice graphs like virt-manager does?
>>
>> It would be nice if we have an example to inspire people. And that's
>> what domtop should do. Yes, it could be written in different ways, but
>> I've chosen this one as I think it show explicitly what users need to
>> implement in order to imitate virt-manager's graphing.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  .gitignore  |   1 +
>>  Makefile.am |   2 +-
>>  cfg.mk  |   2 +-
>>  configure.ac|   1 +
>>  examples/domtop/Makefile.am |  27 +++
>>  examples/domtop/domtop.c| 388 
>> 
>>  libvirt.spec.in |   2 +-
>>  7 files changed, 420 insertions(+), 3 deletions(-)
>>  create mode 100644 examples/domtop/Makefile.am
>>  create mode 100644 examples/domtop/domtop.c
>>
> 
> [first round review - I still plan to compile and examine what happens
> when actually running the program, which may result in more comments...]

I was a bit confused that the usage depends on whether I supply a domain
name or not (it looks like without a name, you just list available names
and quit immediately; with a name, you show stats on just that named
domain).  Not sure if the help text could be enhanced to explain that.

I noticed that even though my guest only has one vcpu assigned, the
stats shown listed four cpus (for my test machine), so this is reporting
the cumulative usage of each host cpu, and not of the guest vcpu.
Definitely worth mentioning what perspective the stats are taken from.

The output is hard to visually break apart - either put a blank line
between output spurts (with the current one line per cpu during the
spurt), or put all cpu stats on a single line per spurt. (Oh, I see Jan
also suggested single line per spurt, so it becomes easier to track
columns for usage patterns).

Looking forward to v2.

-- 
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] esx: Fix a comment about VSphere versions

2014-07-17 Thread Matthias Bolte
2014-07-17 0:25 GMT+02:00 Geoff Hickey :
> Update the VSphere version comment in esx_vi.c for ESX 5.1 and 5.5.
> ---
>  src/esx/esx_vi.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)

ACK and pushed!

-- 
Matthias Bolte
http://photron.blogspot.com

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


Re: [libvirt] [PATCH 2/2] storage: Disallow vol_wipe for sparse logical volumes

2014-07-17 Thread Eric Blake
On 07/17/2014 12:10 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1091866
> 
> Add a new boolean 'sparse'.  This will be used by the logical backend
> storage driver to determine whether the target volume is sparse or not
> (also known by a snapshot or thin logical volume). Although setting sparse
> to true at creation could be seen as duplicitous to setting during
> virStorageBackendLogicalMakeVol() in case there are ever other code paths
> between Create and FindLVs that need to know about the volume be sparse.
> 
> Use the 'sparse' in a new virStorageBackendLogicalVolWipe() to decide whether
> to attempt to wipe the logical volume or not. For now, I have found no
> means to wipe the volume without writing to it. Writing to the sparse
> volume causes it to be filled. A sparse logical volume is not complely

s/complely/completely/

> writeable as there exists metadata which if overwritten will cause the
> sparse lv to go INACTIVE which means pool-refresh will not find it.
> Access to whatever lvm uses to manage data blocks is not provided by
> any API I could find.
> 

I wonder if lvm developers could point us to an alternative; maybe even
lvreduce (shrink the allocated size) followed by lvresize (regrow it,
but the new growth starts life empty) might behave semantically like a
wipe.  But yeah, we don't need to solve that today.

> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend_logical.c | 39 
> ++-
>  src/util/virstoragefile.h |  1 +
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 

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 1/2] storage: Convert 'building' into a bool

2014-07-17 Thread Eric Blake
On 07/17/2014 12:10 PM, John Ferlan wrote:
> Rather than a unsigned int, use a 'bool' since that's how it was used.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/storage_conf.h  | 2 +-
>  src/storage/storage_driver.c | 8 
>  2 files changed, 5 insertions(+), 5 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

[libvirt] [PATCH 0/2] Disallow wiping a sparse logical volume

2014-07-17 Thread John Ferlan
The first patch just changes 'building' into a bool as that's how I
found it used when going to add a new bool for patch 2

Patch 2 addresses the following bz:

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

Essentially, wiping the lv caused it to disappear after a vol-refresh.
This was because the lv that was created was a sparse (or thin or
snapshot via --virtualsize of -V) logical volume.  The multiple names
are the ways I found the feature described.  When the sparse volume
was written to by the wipe algorithms it actually filled the volume
beyond it's capacity rendering it INACTIVE which causes pool-refresh
to ignore it. As it turns out a sparse lv created of size (for example)
4 MiB has metadata contained within the size created. While we could
adjust the size of the metadata, control over it's location becomes
the issue.  It seems there is also some guard data at each end of the
sparse lv as when the wipe was done lost I/O messages were sent to
the system log.

After much searching it just seems that writing a sparse lv with some
sort of wipe algorithm will not work. Although if anyone has suggestions
I'm willing to try. I have found writing '0x00''s, the scrub utility,
and a "dd if=/dev/zero of=/dev/lv_pool/lv_test bs=4096 count=1024"
doesn't work.

It's also of interest to note that the 'scrub' utility doesn't seem
to recognize the logical volume format properly as none of the nonzero
algorithms are run - each fails to open the file - for example:

$ /usr/sbin/lvcreate --name lv_test -L 4096K lv_pool
$ /usr/bin/scrub -f -p bsi /dev/lv_pool/lv_test
scrub: using BSI patterns
scrub: warning: /dev/lv_pool/lv_test is zero length
$ 


John Ferlan (2):
  storage: Convert 'building' into a bool
  storage: Disallow vol_wipe for sparse logical volumes

 src/conf/storage_conf.h   |  2 +-
 src/storage/storage_backend_logical.c | 39 ++-
 src/storage/storage_driver.c  |  8 +++
 src/util/virstoragefile.h |  1 +
 4 files changed, 44 insertions(+), 6 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCH 1/2] storage: Convert 'building' into a bool

2014-07-17 Thread John Ferlan
Rather than a unsigned int, use a 'bool' since that's how it was used.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.h  | 2 +-
 src/storage/storage_driver.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index badf7a3..54c978d 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -63,7 +63,7 @@ struct _virStorageVolDef {
 char *key;
 int type; /* virStorageVolType */
 
-unsigned int building;
+bool building;
 unsigned int in_use;
 
 virStorageVolSource source;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 441da21..3830867 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1691,7 +1691,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
 
 /* Drop the pool lock during volume allocation */
 pool->asyncjobs++;
-voldef->building = 1;
+voldef->building = true;
 virStoragePoolObjUnlock(pool);
 
 buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags);
@@ -1700,7 +1700,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
 virStoragePoolObjLock(pool);
 storageDriverUnlock(driver);
 
-voldef->building = 0;
+voldef->building = false;
 pool->asyncjobs--;
 
 if (buildret < 0) {
@@ -1858,7 +1858,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
 
 /* Drop the pool lock during volume allocation */
 pool->asyncjobs++;
-newvol->building = 1;
+newvol->building = true;
 origvol->in_use++;
 virStoragePoolObjUnlock(pool);
 
@@ -1876,7 +1876,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
 storageDriverUnlock(driver);
 
 origvol->in_use--;
-newvol->building = 0;
+newvol->building = false;
 allocation = newvol->target.allocation;
 pool->asyncjobs--;
 
-- 
1.9.3

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


[libvirt] [PATCH 2/2] storage: Disallow vol_wipe for sparse logical volumes

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

Add a new boolean 'sparse'.  This will be used by the logical backend
storage driver to determine whether the target volume is sparse or not
(also known by a snapshot or thin logical volume). Although setting sparse
to true at creation could be seen as duplicitous to setting during
virStorageBackendLogicalMakeVol() in case there are ever other code paths
between Create and FindLVs that need to know about the volume be sparse.

Use the 'sparse' in a new virStorageBackendLogicalVolWipe() to decide whether
to attempt to wipe the logical volume or not. For now, I have found no
means to wipe the volume without writing to it. Writing to the sparse
volume causes it to be filled. A sparse logical volume is not complely
writeable as there exists metadata which if overwritten will cause the
sparse lv to go INACTIVE which means pool-refresh will not find it.
Access to whatever lvm uses to manage data blocks is not provided by
any API I could find.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_logical.c | 39 ++-
 src/util/virstoragefile.h |  1 +
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index ab1e64b..ed62c2f 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -131,6 +131,15 @@ virStorageBackendLogicalMakeVol(char **const groups,
 goto cleanup;
 }
 
+/* Mark the (s) sparse/snapshot lv, e.g. the lv created using
+ * the --virtualsize/-V option. We've already ignored the (t)hin
+ * pool definition. In the manner libvirt defines these, the
+ * thin pool is hidden to the lvs output, except as the name
+ * in brackets [] described for the groups[1] (backingStore).
+ */
+if (attrs[0] == 's')
+vol->target.sparse = true;
+
 /* Skips the backingStore of lv created with "--virtualsize",
  * its original device "/dev/$vgname/$lvname_vorigin" is
  * just for lvm internal use, one should never use it.
@@ -752,6 +761,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
VIR_DIV_UP(vol->target.allocation
   ? vol->target.allocation : 1, 1024));
 virCommandAddArg(cmd, "--virtualsize");
+vol->target.sparse = true;
 }
 virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity,
 1024));
@@ -829,6 +839,33 @@ virStorageBackendLogicalBuildVolFrom(virConnectPtr conn,
 return build_func(conn, pool, vol, inputvol, flags);
 }
 
+static int
+virStorageBackendLogicalVolWipe(virConnectPtr conn,
+virStoragePoolObjPtr pool,
+virStorageVolDefPtr vol,
+unsigned int algorithm,
+unsigned int flags)
+{
+if (!vol->target.sparse)
+return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, 
flags);
+
+/* The wiping algorithms will write something to the logical volume.
+ * Writing to a sparse logical volume causes it to be filled resulting
+ * in the volume becoming INACTIVE because there is some amount of
+ * metadata contained within the sparse lv. Choosing to only write
+ * a wipe pattern to the already written portion lv based on what
+ * 'lvs' shows in the "Data%" column/field for the sparse lv was
+ * considered. However, there is no guarantee that sparse lv could
+ * grow or shrink outside of libvirt's knowledge and thus still render
+ * the volume INACTIVE. Until there is some sort of wipe function
+ * implemented by lvm for one of these sparse lv, we'll just return
+ * unsupported.
+ */
+virReportError(VIR_ERR_NO_SUPPORT,
+   _("logical volue '%s' is sparse, volume wipe not 
supported"),
+   vol->target.path);
+return -1;
+}
 
 virStorageBackend virStorageBackendLogical = {
 .type = VIR_STORAGE_POOL_LOGICAL,
@@ -846,5 +883,5 @@ virStorageBackend virStorageBackendLogical = {
 .deleteVol = virStorageBackendLogicalDeleteVol,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
-.wipeVol = virStorageBackendVolWipeLocal,
+.wipeVol = virStorageBackendLogicalVolWipe,
 };
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c840aa0..eccbf4e 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -248,6 +248,7 @@ struct _virStorageSource {
 virBitmapPtr features;
 char *compat;
 bool nocow;
+bool sparse;
 
 virStoragePermsPtr perms;
 virStorageTimestampsPtr timestamps;
-- 
1.9.3

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


Re: [libvirt] [PATCH v3 07/16] numatune: Encapsulate numatune configuration in order to unify results

2014-07-17 Thread Roman Bogorodskiy
  Eric Blake wrote:

> On 07/17/2014 10:57 AM, Roman Bogorodskiy wrote:
> >   Roman Bogorodskiy wrote:
> > 
> 
> >> Looks like this breaks build with clang:
> >>
> >> gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
> >>   CC   util/libvirt_util_la-virclosecallbacks.lo
> >> In file included from util/virclosecallbacks.c:28:
> >> In file included from ../src/util/virclosecallbacks.h:28:
> >> ../src/conf/domain_conf.h:70:35: error: redefinition of typedef 
> >> 'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
> >> typedef struct _virDomainNumatune virDomainNumatune;
> >>   ^
> >> ../src/conf/numatune_conf.h:43:35: note: previous definition is here
> >> typedef struct _virDomainNumatune virDomainNumatune;
> >>   ^
> 
> > 
> > I got it fixed by the following diff:
> > 
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 4c9b7e8..e4d7988 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -67,9 +67,6 @@ typedef virDomainFSDef *virDomainFSDefPtr;
> >  typedef struct _virDomainNetDef virDomainNetDef;
> >  typedef virDomainNetDef *virDomainNetDefPtr;
> >  
> > -typedef struct _virDomainNumatune virDomainNumatune;
> > -typedef virDomainNumatune *virDomainNumatunePtr;
> > -
> >  typedef struct _virDomainInputDef virDomainInputDef;
> >  typedef virDomainInputDef *virDomainInputDefPtr;
> 
> ACK to this hunk.
> 
> >  
> > @@ -1854,8 +1851,6 @@ struct _virDomainResourceDef {
> >   * NB: if adding to this struct, virDomainDefCheckABIStability
> >   * may well need an update
> >   */
> > -typedef struct _virDomainDef virDomainDef;
> > -typedef virDomainDef *virDomainDefPtr;
> >  struct _virDomainDef {
> 
> But this hunk feels fishy.  Why does numatune_conf.h need virDomainDef?
>  It's a leaky abstraction - virDomainNumatuneNodeParseXML is accessing
> def->cpu directly instead of limiting itself to just def->numatune.
> Also, virDomainNumatuneParseXML is accessing def->placement_mode, which
> argues that placement_mode should be part of def->numatune rather than
> an independent variable.
> 
> Yes, this hunk solves the compiler fix, but it means that we are just
> cementing that we didn't abstract things into a single object.  That is,
> my ideal setup would be that numatune has access to all the pieces that
> it reads/modifies as parameters, but not access the entire virDomainDef,
> and then we don't have a circular referencing situation and don't need
> numatune_conf.h to be the source of our typedef declaration of virDomainDef.
> 
> > I didn't check it beyond build and check/syntax-check though. Anyway, it
> > doesn't look quite clean to have typedefs in numatune_conf.h for the
> > struct declared in domain_conf.h.
> 
> I'd be fine with your patch going in as a stop-gap compilation fix, even
> if I still think that we could restructure the code better to make
> numatune_conf.h self-contained and move the typedef for virDomainDef
> back to domain_conf.h.

Good, so I pushed the patch.

Roman Bogorodskiy


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

Re: [libvirt] [PATCH 0/2] virsh: negative numbers for specific commands

2014-07-17 Thread John Ferlan


On 07/16/2014 07:35 PM, John Ferlan wrote:
> Following up to the recently restarted discussion:
> 
> http://www.redhat.com/archives/libvir-list/2014-July/msg00686.html
> 
> regarding negative values for certain virsh commands - these patches
> will document the "feature" of using a negative value to indicate
> the largest value *and* for the vol-{upload|download} change 'offset'
> to not accept a negative value.
> 
> John Ferlan (2):
>   virsh vol-upload/download disallow negative offset
>   virsh: Document bandwidth maximum more clearly
> 
>  tools/virsh-volume.c |  6 +++---
>  tools/virsh.pod  | 33 +++--
>  2 files changed, 30 insertions(+), 9 deletions(-)
> 

Thanks - pushed.

John

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


Re: [libvirt] [PATCH v3 00/16] Support for per-guest-node binding

2014-07-17 Thread Eric Blake
On 07/17/2014 01:37 AM, Ján Tomko wrote:
> On 07/16/2014 04:42 PM, Martin Kletzander wrote:
>> v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html
>>

> 
> This series broke the build for me (clang 3.4.1):
> In file included from util/virclosecallbacks.c:28:
> In file included from ../src/util/virclosecallbacks.h:28:
> ../src/conf/domain_conf.h:70:35: error: redefinition of typedef
> 'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
> typedef struct _virDomainNumatune virDomainNumatune;
>   ^
> ../src/conf/numatune_conf.h:43:35: note: previous definition is here
> typedef struct _virDomainNumatune virDomainNumatune;

Roman hit the same; see the sub-thread at 7/16.

-- 
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 v3 07/16] numatune: Encapsulate numatune configuration in order to unify results

2014-07-17 Thread Eric Blake
On 07/17/2014 10:57 AM, Roman Bogorodskiy wrote:
>   Roman Bogorodskiy wrote:
> 

>> Looks like this breaks build with clang:
>>
>> gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
>>   CC   util/libvirt_util_la-virclosecallbacks.lo
>> In file included from util/virclosecallbacks.c:28:
>> In file included from ../src/util/virclosecallbacks.h:28:
>> ../src/conf/domain_conf.h:70:35: error: redefinition of typedef 
>> 'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
>> typedef struct _virDomainNumatune virDomainNumatune;
>>   ^
>> ../src/conf/numatune_conf.h:43:35: note: previous definition is here
>> typedef struct _virDomainNumatune virDomainNumatune;
>>   ^

> 
> I got it fixed by the following diff:
> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4c9b7e8..e4d7988 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -67,9 +67,6 @@ typedef virDomainFSDef *virDomainFSDefPtr;
>  typedef struct _virDomainNetDef virDomainNetDef;
>  typedef virDomainNetDef *virDomainNetDefPtr;
>  
> -typedef struct _virDomainNumatune virDomainNumatune;
> -typedef virDomainNumatune *virDomainNumatunePtr;
> -
>  typedef struct _virDomainInputDef virDomainInputDef;
>  typedef virDomainInputDef *virDomainInputDefPtr;

ACK to this hunk.

>  
> @@ -1854,8 +1851,6 @@ struct _virDomainResourceDef {
>   * NB: if adding to this struct, virDomainDefCheckABIStability
>   * may well need an update
>   */
> -typedef struct _virDomainDef virDomainDef;
> -typedef virDomainDef *virDomainDefPtr;
>  struct _virDomainDef {

But this hunk feels fishy.  Why does numatune_conf.h need virDomainDef?
 It's a leaky abstraction - virDomainNumatuneNodeParseXML is accessing
def->cpu directly instead of limiting itself to just def->numatune.
Also, virDomainNumatuneParseXML is accessing def->placement_mode, which
argues that placement_mode should be part of def->numatune rather than
an independent variable.

Yes, this hunk solves the compiler fix, but it means that we are just
cementing that we didn't abstract things into a single object.  That is,
my ideal setup would be that numatune has access to all the pieces that
it reads/modifies as parameters, but not access the entire virDomainDef,
and then we don't have a circular referencing situation and don't need
numatune_conf.h to be the source of our typedef declaration of virDomainDef.

> I didn't check it beyond build and check/syntax-check though. Anyway, it
> doesn't look quite clean to have typedefs in numatune_conf.h for the
> struct declared in domain_conf.h.

I'd be fine with your patch going in as a stop-gap compilation fix, even
if I still think that we could restructure the code better to make
numatune_conf.h self-contained and move the typedef for virDomainDef
back to domain_conf.h.

-- 
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 v3 07/16] numatune: Encapsulate numatune configuration in order to unify results

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

>   Martin Kletzander wrote:
> 
> > There were numerous places where numatune configuration (and thus
> > domain config as well) was changed in different ways.  On some
> > places this even resulted in persistent domain definition not to be
> > stable (it would change with daemon's restart).
> > 
> > In order to uniformly change how numatune config is dealt with, all
> > the internals are now accessible directly only in numatune_conf.c and
> > outside this file accessors must be used.
> > 
> > Signed-off-by: Martin Kletzander 
> > ---
> >  po/POTFILES.in |   1 +
> >  src/conf/domain_conf.c | 159 ++-
> >  src/conf/domain_conf.h |   8 +-
> >  src/conf/numatune_conf.c   | 318 
> > +
> >  src/conf/numatune_conf.h   |  72 -
> >  src/libvirt_private.syms   |  11 +
> >  src/lxc/lxc_cgroup.c   |  19 +-
> >  src/lxc/lxc_controller.c   |   5 +-
> >  src/lxc/lxc_native.c   |  15 +-
> >  src/parallels/parallels_driver.c   |   7 +-
> >  src/qemu/qemu_cgroup.c |  23 +-
> >  src/qemu/qemu_driver.c |  84 +++---
> >  src/qemu/qemu_process.c|   8 +-
> >  src/util/virnuma.c |  48 ++--
> >  src/util/virnuma.h |   2 +-
> >  .../qemuxml2argv-numatune-auto-prefer.xml  |  29 ++
> >  .../qemuxml2xmlout-numatune-auto-prefer.xml|  29 ++
> >  tests/qemuxml2xmltest.c|   2 +
> >  18 files changed, 555 insertions(+), 285 deletions(-)
> >  create mode 100644 
> > tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
> >  create mode 100644 
> > tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
> 
> Looks like this breaks build with clang:
> 
> gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
>   CC   util/libvirt_util_la-virclosecallbacks.lo
> In file included from util/virclosecallbacks.c:28:
> In file included from ../src/util/virclosecallbacks.h:28:
> ../src/conf/domain_conf.h:70:35: error: redefinition of typedef 
> 'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
> typedef struct _virDomainNumatune virDomainNumatune;
>   ^
> ../src/conf/numatune_conf.h:43:35: note: previous definition is here
> typedef struct _virDomainNumatune virDomainNumatune;
>   ^
> In file included from util/virclosecallbacks.c:28:
> In file included from ../src/util/virclosecallbacks.h:28:
> ../src/conf/domain_conf.h:71:28: error: redefinition of typedef 
> 'virDomainNumatunePtr' is a C11 feature [-Werror,-Wtypedef-redefinition]
> typedef virDomainNumatune *virDomainNumatunePtr;
>^
> ../src/conf/numatune_conf.h:44:28: note: previous definition is here
> typedef virDomainNumatune *virDomainNumatunePtr;
>^
> In file included from util/virclosecallbacks.c:28:
> In file included from ../src/util/virclosecallbacks.h:28:
> ../src/conf/domain_conf.h:1857:30: error: redefinition of typedef 
> 'virDomainDef' is a C11 feature [-Werror,-Wtypedef-redefinition]
> typedef struct _virDomainDef virDomainDef;
>  ^
> ../src/conf/numatune_conf.h:39:30: note: previous definition is here
> typedef struct _virDomainDef virDomainDef;
>  ^
> In file included from util/virclosecallbacks.c:28:
> In file included from ../src/util/virclosecallbacks.h:28:
> ../src/conf/domain_conf.h:1858:23: error: redefinition of typedef 
> 'virDomainDefPtr' is a C11 feature [-Werror,-Wtypedef-redefinition]
> typedef virDomainDef *virDomainDefPtr;
>   ^
> ../src/conf/numatune_conf.h:40:23: note: previous definition is here
> typedef virDomainDef *virDomainDefPtr;
>   ^
> 4 errors generated.
> 
> Should we probably drop the repeating definitions from domain_conf.h as
> we're including numatune_conf.h anyway?

I got it fixed by the following diff:

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4c9b7e8..e4d7988 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -67,9 +67,6 @@ typedef virDomainFSDef *virDomainFSDefPtr;
 typedef struct _virDomainNetDef virDomainNetDef;
 typedef virDomainNetDef *virDomainNetDefPtr;
 
-typedef struct _virDomainNumatune virDomainNumatune;
-typedef virDomainNumatune *virDomainNumatunePtr;
-
 typedef struct _virDomainInputDef virDomainInputDef;
 typedef virDomainInputDef *virDomainInputDefPtr;
 
@@ -1854,8 +1851,6 @@ struct _virDomainResourceDef {
  * NB: if adding to this struct, virDomainDefCheckABIStability
  * may well need an update
  */
-typedef struct _virDomai

Re: [libvirt] [PATCH v3 07/16] numatune: Encapsulate numatune configuration in order to unify results

2014-07-17 Thread Roman Bogorodskiy
  Martin Kletzander wrote:

> There were numerous places where numatune configuration (and thus
> domain config as well) was changed in different ways.  On some
> places this even resulted in persistent domain definition not to be
> stable (it would change with daemon's restart).
> 
> In order to uniformly change how numatune config is dealt with, all
> the internals are now accessible directly only in numatune_conf.c and
> outside this file accessors must be used.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  po/POTFILES.in |   1 +
>  src/conf/domain_conf.c | 159 ++-
>  src/conf/domain_conf.h |   8 +-
>  src/conf/numatune_conf.c   | 318 
> +
>  src/conf/numatune_conf.h   |  72 -
>  src/libvirt_private.syms   |  11 +
>  src/lxc/lxc_cgroup.c   |  19 +-
>  src/lxc/lxc_controller.c   |   5 +-
>  src/lxc/lxc_native.c   |  15 +-
>  src/parallels/parallels_driver.c   |   7 +-
>  src/qemu/qemu_cgroup.c |  23 +-
>  src/qemu/qemu_driver.c |  84 +++---
>  src/qemu/qemu_process.c|   8 +-
>  src/util/virnuma.c |  48 ++--
>  src/util/virnuma.h |   2 +-
>  .../qemuxml2argv-numatune-auto-prefer.xml  |  29 ++
>  .../qemuxml2xmlout-numatune-auto-prefer.xml|  29 ++
>  tests/qemuxml2xmltest.c|   2 +
>  18 files changed, 555 insertions(+), 285 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml

Looks like this breaks build with clang:

gmake[3]: Entering directory `/usr/home/novel/code/libvirt/src'
  CC   util/libvirt_util_la-virclosecallbacks.lo
In file included from util/virclosecallbacks.c:28:
In file included from ../src/util/virclosecallbacks.h:28:
../src/conf/domain_conf.h:70:35: error: redefinition of typedef 
'virDomainNumatune' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct _virDomainNumatune virDomainNumatune;
  ^
../src/conf/numatune_conf.h:43:35: note: previous definition is here
typedef struct _virDomainNumatune virDomainNumatune;
  ^
In file included from util/virclosecallbacks.c:28:
In file included from ../src/util/virclosecallbacks.h:28:
../src/conf/domain_conf.h:71:28: error: redefinition of typedef 
'virDomainNumatunePtr' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef virDomainNumatune *virDomainNumatunePtr;
   ^
../src/conf/numatune_conf.h:44:28: note: previous definition is here
typedef virDomainNumatune *virDomainNumatunePtr;
   ^
In file included from util/virclosecallbacks.c:28:
In file included from ../src/util/virclosecallbacks.h:28:
../src/conf/domain_conf.h:1857:30: error: redefinition of typedef 
'virDomainDef' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct _virDomainDef virDomainDef;
 ^
../src/conf/numatune_conf.h:39:30: note: previous definition is here
typedef struct _virDomainDef virDomainDef;
 ^
In file included from util/virclosecallbacks.c:28:
In file included from ../src/util/virclosecallbacks.h:28:
../src/conf/domain_conf.h:1858:23: error: redefinition of typedef 
'virDomainDefPtr' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef virDomainDef *virDomainDefPtr;
  ^
../src/conf/numatune_conf.h:40:23: note: previous definition is here
typedef virDomainDef *virDomainDefPtr;
  ^
4 errors generated.

Should we probably drop the repeating definitions from domain_conf.h as
we're including numatune_conf.h anyway?

Roman Bogorodskiy


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

Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events

2014-07-17 Thread Eric Blake
On 07/17/2014 09:18 AM, Peter Krempa wrote:

>>> A second issue that comes into my mind is compatibility with already
>>> existing files. Does this work when you already have a lease file? (I
>>> didn't try it, I'm just asking).
>>
>> If we use the --leasefile-ro option, then although this method will
>> work, but no, the lease file will *not* be read/written at all. So
>> even if an old one exists, its of no use.
>> But then again, the use of --leasefile-ro is mandatory, so that all
>> events are captured by our helper program. For example, renew of a
>> lease.
> 
> My concerns are whether this will work in the case you already used the
> leases helper as the patch is adding a few fields to the stored format.

In particular, this scenario MUST work:

A user installs libvirt 1.2.6, and starts a guest (which creates a
leases file and longrunning helper app).  Then the user upgrades to
libvirt 1.2.7 and restarts libvirtd.  The new libvirt MUST be able to
interact with the running guest that is still using the older-style
leases file and helper app, with no loss of behavior to the guest.

Also, this scenario MUST work:

A user installs libvirt 1.2.6.  Then they upgrade to libvirt 1.2.7, but
without restarting libvirtd.  Then they start a guest.  Note that the
upgrade means that the leaseshelper application that will be started is
the 1.2.7 build, but the command line to start it will be triggered by
the 1.2.6 libvirtd.  The new helper must do everything that the old
helper did, so that the old libvirtd does not have any hiccups in operation.

The easiest way to ensure that both directions of version mismatch are
well-behaved is to make sure that either side can be newer than the
other, that the changes are purely additive in nature, and that there
are sane defaults in place in the newer code when dealing with the older
data that lacks the additions.  Also, while upgrade is a required
scenario, downgrade generally is not (going from 1.2.7 back to 1.2.6
while a guest is still running, or expecting 1.2.7 libvirtd to be able
to use 1.2.6 leaseshelper, is nice if it works but not a showstopper if
it doesn't).

-- 
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] virsh vol-upload/download disallow negative offset

2014-07-17 Thread Eric Blake
On 07/17/2014 09:25 AM, Peter Krempa wrote:
> On 07/17/14 01:35, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1087104
>>
>> Commit id 'c6212539' explicitly allowed a negative value to be used for
>> offset and length as a shorthand for the largest value after commit id
>> 'f18c02ec' modified virStrToLong_ui() to essentially disallow a negative
>> value.
>>
>> However, allowing a negative value for offset ONLY worked if the negative
>> value was -1 since the eventual lseek() does allow a -1 to mean the end
>> of the file.  Providing other negative values resulted in errors such as:

lseek to -1 is not necessarily the end of the file.  But I'm not sure
what a better wording of this part of the commit message would be.

>>
>> $ virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw \
>>   --offset -2 --length -1000
>> error: cannot download from volume qcow3-vol2
>> error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: 
>> Invalid argument
>>
>> $
>>
>> Thus, it seems unreasonable to expect or allow a negative value for offset
>> since the only benefit is to lseek() to the end of the file and then only
>> take advantage of how the OS would handle such a seek. For the purposes of
>> upload or download of volume data, that seems to be a no-op.  Therefore,
>> disallow a negative value for offset.
>>
>> Additionally, modify the man page for vol-upload and vol-download to provide
>> more details regarding the valid values for both offset and length.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  tools/virsh-volume.c |  6 +++---
>>  tools/virsh.pod  | 10 --
>>  2 files changed, 11 insertions(+), 5 deletions(-)
>>
> 
> ACK, although I'd prefer Eric stating his opinion on this too.
> 

At any rate, the code and documentation of this patch are fine with me -
keeping the negative length as shorthand to catch the rest of the file
is fine.  And if we ever want to add negative offset as distance from
the end of the file, we can still do that in the future; but disabling
it for now is fine.

I think the ACK stands.

-- 
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 v1 3/7] qemu: Utilize virFileFindHugeTLBFS

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

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

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am  |   1 +
 src/qemu/qemu_command.c  |  20 
 src/qemu/qemu_conf.c | 122 +--
 src/qemu/qemu_conf.h |   9 +++-
 src/qemu/qemu_driver.c   |  39 +++
 src/qemu/qemu_process.c  |  21 +---
 tests/qemuxml2argvtest.c |  10 ++--
 7 files changed, 166 insertions(+), 56 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 982f63d..85e61c2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1212,6 +1212,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
 $(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(LIBXML_LIBS) \
+   libvirt_util.la \
$(NULL)
 libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8062510..b14ce83 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7333,14 +7333,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024;
 virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024);
 if (def->mem.hugepage_backed) {
-if (!cfg->hugetlbfsMount) {
+char *mem_path;
+
+if (!cfg->nhugetlbfs) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("hugetlbfs filesystem is not mounted"));
-goto error;
-}
-if (!cfg->hugepagePath) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("hugepages are disabled by administrator 
config"));
+   "%s", _("hugetlbfs filesystem is not mounted "
+   "or disabled by administrator config"));
 goto error;
 }
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
@@ -7349,8 +7347,14 @@ qemuBuildCommandLine(virConnectPtr conn,
def->emulator);
 goto error;
 }
+
+if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs,
+cfg->nhugetlbfs)))
+goto error;
+
 virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
- cfg->hugepagePath, NULL);
+ mem_path, NULL);
+VIR_FREE(mem_path);
 }
 
 if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e62bec0..cf5ce97 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -230,19 +230,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 cfg->migrationPortMin = QEMU_MIGRATION_PORT_MIN;
 cfg->migrationPortMax = QEMU_MIGRATION_PORT_MAX;
 
-#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
-/* For privileged driver, try and find hugepage mount automatically.
+/* For privileged driver, try and find hugetlbfs mounts automatically.
  * Non-privileged driver requires admin to create a dir for the
- * user, chown it, and then let user configure it manually */
+ * user, chown it, and then let user configure it manually. */
 if (privileged &&
-!(cfg->hugetlbfsMount = virFileFindMountPoint("hugetlbfs"))) {
-if (errno != ENOENT) {
-virReportSystemError(errno, "%s",
- _("unable to find hugetlbfs mountpoint"));
-goto error;
-}
-}
-#endif
+virFileFindHugeTLBFS(&cfg->hugetlbfs, &cfg->nhugetlbfs) < 0)
+goto error;
+
 if (VIR_STRDUP(cfg->bridgeHelperName, "/usr/libexec/qemu-bridge-helper") < 
0)
 goto error;
 
@@ -293,8 +287,11 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->spicePassword);
 VIR_FREE(cfg->spiceSASLdir);
 
-VIR_FREE(cfg->hugetlbfsMount);
-VIR_FREE(cfg->hugepagePath);
+while (cfg->nhugetlbfs) {
+cfg->nhugetlbfs--;
+VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+}
+VIR_FREE(cfg->hugetlbfs);
 VIR_FREE(cfg->bridgeHelperName);
 
 VIR_FREE(cfg->saveImageFormat);
@@ -307,6 +304,26 @@ static void virQEMUDriverConfigDispose(void *obj)
 }
 
 
+static int
+virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
+ const char *path,
+ bool deflt)
+{
+int ret = -1;
+
+ 

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

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

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

[libvirt] [PATCH v1 5/7] domain: Introduce ./hugepages/page/[@size, @unit, @nodeset]

2014-07-17 Thread Michal Privoznik
  

  
  

  

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

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3c85fc5..f4362e6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -617,7 +617,9 @@
 
   ...
   
-
+
+  
+  
 
 
   
@@ -632,7 +634,19 @@
 
   hugepages
   This tells the hypervisor that the guest should have its memory
-allocated using hugepages instead of the normal native page size.
+  allocated using hugepages instead of the normal native page size.
+  Since 1.2.5 it's possible to set hugepages
+  more specifically per numa node. The page element is
+  introduced. It has one compulsory attribute size which
+  specifies which hugepages should be used (especially useful on systems
+  supporting hugepages of different sizes). The default unit for the
+  size attribute is kilobytes (multiplier of 1024). If you
+  want to use different unit, use optional unit attribute.
+  For systems with NUMA, the optional nodeset attribute may
+  come handy as it ties given guest's NUMA nodes to certain hugepage
+  sizes. From the example snippet, one gigabyte hugepages are used for
+  every NUMA node except node number four. For the correct syntax see
+  this.
   nosharepages
   Instructs hypervisor to disable shared pages (memory merge, KSM) for
 this domain. Since 1.0.6
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2caeef9..d9da0bc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -567,7 +567,24 @@
   
 
   
-
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1ef374..b49bcb0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11258,6 +11258,57 @@ virDomainParseMemory(const char *xpath, 
xmlXPathContextPtr ctxt,
 }
 
 
+static int
+virDomainHugepagesParseXML(xmlNodePtr node,
+   xmlXPathContextPtr ctxt,
+   virDomainHugePagePtr hugepage)
+{
+int ret = -1;
+xmlNodePtr oldnode = ctxt->node;
+unsigned long long bytes, max;
+char *unit = NULL, *nodeset = NULL;
+
+ctxt->node = node;
+
+/* On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63).  */
+if (sizeof(unsigned long) < sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
+if (virXPathULongLong("string(./@size)", ctxt, &bytes) < 0) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+   _("unable to parse size attribute"));
+goto cleanup;
+}
+
+unit = virXPathString("string(./@unit)", ctxt);
+
+if (virScaleInteger(&bytes, unit, 1024, max) < 0)
+goto cleanup;
+
+if (!(hugepage->size = VIR_DIV_UP(bytes, 1024))) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+   _("hugepage size can't be zero"));
+goto cleanup;
+}
+
+if ((nodeset = virXMLPropString(node, "nodeset"))) {
+if (virBitmapParse(nodeset, 0, &hugepage->nodemask,
+   VIR_DOMAIN_CPUMASK_LEN) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(unit);
+VIR_FREE(nodeset);
+ctxt->node = oldnode;
+return ret;
+}
+
+
 static virDomainResourceDefPtr
 virDomainResourceDefParse(xmlNodePtr node,
   xmlXPathContextPtr ctxt)
@@ -11325,7 +11376,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 {
 xmlNodePtr *nodes

[libvirt] [PATCH v1 7/7] tests: Some testing of hugepages mapping

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

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
new file mode 100644
index 000..9211bc6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
@@ -0,0 +1,10 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 1024 -smp 2 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=256M,id=ram-node0 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu,size=768M,id=ram-node1 \
+-numa node,nodeid=1,cpus=1,memdev=ram-node1 \
+-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
new file mode 100644
index 000..3df870b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
@@ -0,0 +1,38 @@
+
+  SomeDummyHugepagesGuest
+  ef1bdff4-27f3-4e85-a807-5fb4d58463cc
+  1048576
+  1048576
+  
+
+  
+
+  
+  2
+  
+hvm
+
+  
+  
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
new file mode 100644
index 000..27b3f8e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
@@ -0,0 +1,9 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 1024 -smp 2 \
+-object memory-backend-ram,size=256M,id=ram-node0 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-object memory-backend-file,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu,size=768M,id=ram-node1 \
+-numa node,nodeid=1,cpus=1,memdev=ram-node1 \
+-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
\
+-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
new file mode 100644
index 000..35aa2cf
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
@@ -0,0 +1,38 @@
+
+  SomeDummyHugepagesGuest
+  ef1bdff4-27f3-4e85-a807-5fb4d58463cc
+  1048576
+  1048576
+  
+
+  
+
+  
+  2
+  
+hvm
+
+  
+  
+
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 63c9c4b..b4505a9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -669,6 +669,10 @@ mymain(void)
 DO_TEST("hugepages", QEMU_CAPS_MEM_PATH);
 DO_TEST("hugepages-pages", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_RAM,
 QEMU_CAPS_OBJECT_MEMORY_FILE);
+DO_TEST("hugepages-pages2", QEMU_CAPS_MEM_PATH, 
QEMU_CAPS_OBJECT_MEMORY_RAM,
+QEMU_CAPS_OBJECT_MEMORY_FILE);
+DO_TEST("hugepages-pages3", QEMU_CAPS_MEM_PATH, 
QEMU_CAPS_OBJECT_MEMORY_RAM,
+QEMU_CAPS_OBJECT_MEMORY_FILE);
 DO_TEST("nosharepages", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MEM_MERGE);
 DO_TEST("disk-cdrom", NONE);
 DO_TEST("disk-cdrom-network-http", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 09cb228..8d46b40 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -198,6 +198,8 @@ mymain(void)
 
 DO_TEST("hugepages");
 DO_TEST("hugepages-pages");
+DO_TEST("hugepages-pages2");
+DO_TEST("hugepages-pages3");
 DO_TEST("nosharepages");
 DO_TEST("disk-aio");
 DO_TEST("disk-cdrom");
-- 
1.8.5.5

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


[libvirt] [PATCH v1 4/7] virbitmap: Introduce virBitmapDoesIntersect

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

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |  1 +
 src/util/virbitmap.c | 20 
 src/util/virbitmap.h |  3 +++
 tests/virbitmaptest.c| 26 ++
 4 files changed, 50 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 44403fd..256edd5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1016,6 +1016,7 @@ virBitmapClearBit;
 virBitmapCopy;
 virBitmapCountBits;
 virBitmapDataToString;
+virBitmapDoesIntersect;
 virBitmapEqual;
 virBitmapFormat;
 virBitmapFree;
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 1029635..9f82eb9 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -732,3 +732,23 @@ virBitmapDataToString(void *data,
 virBitmapFree(map);
 return ret;
 }
+
+bool
+virBitmapDoesIntersect(virBitmapPtr b1,
+   virBitmapPtr b2)
+{
+size_t i;
+
+if (b1->max_bit > b2->max_bit) {
+virBitmapPtr tmp = b1;
+b1 = b2;
+b2 = tmp;
+}
+
+for (i = 0; i < b1->map_len; i++) {
+if (b1->map[i] & b2->map[i])
+return true;
+}
+
+return false;
+}
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 142a218..063516a 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -114,5 +114,8 @@ size_t virBitmapCountBits(virBitmapPtr bitmap)
 char *virBitmapDataToString(void *data,
 int len)
 ATTRIBUTE_NONNULL(1);
+bool virBitmapDoesIntersect(virBitmapPtr b1,
+virBitmapPtr b2)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 #endif
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 048946f..09b40d7 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -510,6 +510,30 @@ test9(const void *opaque ATTRIBUTE_UNUSED)
 }
 
 static int
+test10(const void *opaque ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
+
+if (virBitmapParse("0-3,5-8,11-15", 0, &b1, 20) < 0 ||
+virBitmapParse("4,9,10,16-19", 0, &b2, 20) < 0 ||
+virBitmapParse("15", 0, &b3, 20) < 0)
+goto cleanup;
+
+if (virBitmapDoesIntersect(b1, b2) ||
+virBitmapDoesIntersect(b2, b3) ||
+!virBitmapDoesIntersect(b1, b3))
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virBitmapFree(b1);
+virBitmapFree(b2);
+virBitmapFree(b3);
+return ret;
+}
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -532,6 +556,8 @@ mymain(void)
 ret = -1;
 if (virtTestRun("test9", test9, NULL) < 0)
 ret = -1;
+if (virtTestRun("test10", test10, NULL) < 0)
+ret = -1;
 
 return ret;
 }
-- 
1.8.5.5

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


[libvirt] [PATCH v1 1/7] configure: Check for statfs

2014-07-17 Thread Michal Privoznik
The statfs(2) gets filesystem statistics. Currently, we use it only on
linux, and leave stub to implement on other platforms. But hey, other
platforms (like FreeBSD) have statfs() too. If we check it in
configure we can wider platforms supported. Speaking of FreeBSD, the
headers to include are of course different: sys/param.h and
sys/mount.h on the FreeBSD and sys/statfs.h on the Linux. The header
files are checked too.

Signed-off-by: Michal Privoznik 
---
 configure.ac   |  4 ++--
 src/util/virfile.c | 21 ++---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8001e24..68ebf75 100644
--- a/configure.ac
+++ b/configure.ac
@@ -275,7 +275,7 @@ dnl and various less common threadsafe functions
 AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
   getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
   posix_memalign prlimit regexec sched_getaffinity setgroups setns \
-  setrlimit symlink sysctlbyname getifaddrs])
+  setrlimit symlink sysctlbyname getifaddrs statfs])
 
 dnl Availability of pthread functions. Because of $LIB_PTHREAD, we
 dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD
@@ -316,7 +316,7 @@ dnl Availability of various common headers (non-fatal if 
missing).
 AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \
   sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
   sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
-  libtasn1.h sys/ucred.h sys/mount.h])
+  libtasn1.h sys/ucred.h sys/mount.h sys/param.h sys/statfs.h])
 dnl Check whether endian provides handy macros.
 AC_CHECK_DECLS([htole64], [], [], [[#include ]])
 
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 463064c..699b9f8 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -43,11 +43,15 @@
 # include 
 #endif
 
-#ifdef __linux__
-# if HAVE_LINUX_MAGIC_H
-#  include 
-# endif
+#if HAVE_LINUX_MAGIC_H
+# include 
+#endif
+
+#ifdef HAVE_SYS_STATFS_H
 # include 
+#elif defined HAVE_SYS_PARAM_H && defined HAVE_SYS_MOUNT_H
+# include 
+# include 
 #endif
 
 #if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR
@@ -2817,7 +2821,7 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 }
 
 
-#ifdef __linux__
+#ifdef HAVE_STATFS
 
 # ifndef NFS_SUPER_MAGIC
 #  define NFS_SUPER_MAGIC 0x6969
@@ -2909,14 +2913,17 @@ virFileIsSharedFSType(const char *path,
 
 return 0;
 }
-#else
+
+#else /* ! HAVE_STATFS */
+
 int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED,
   int fstypes ATTRIBUTE_UNUSED)
 {
 /* XXX implement me :-) */
 return 0;
 }
-#endif
+
+#endif /* HAVE_STATFS */
 
 int virFileIsSharedFS(const char *path)
 {
-- 
1.8.5.5

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


[libvirt] [PATCH v1 2/7] Introduce virFileFindHugeTLBFS

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

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |   1 +
 src/util/virfile.c   | 155 +++
 src/util/virfile.h   |  12 
 3 files changed, 168 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d3671c..44403fd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1297,6 +1297,7 @@ virFileDirectFdFlag;
 virFileExists;
 virFileFclose;
 virFileFdopen;
+virFileFindHugeTLBFS;
 virFileFindMountPoint;
 virFileFindResource;
 virFileFindResourceFull;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 699b9f8..e1034b7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2841,6 +2841,9 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 # ifndef CIFS_SUPER_MAGIC
 #  define CIFS_SUPER_MAGIC 0xFF534D42
 # endif
+# ifndef HUGETLBFS_MAGIC
+#  define HUGETLBFS_MAGIC 0x958458f6
+# endif
 
 int
 virFileIsSharedFSType(const char *path,
@@ -2914,6 +2917,33 @@ virFileIsSharedFSType(const char *path,
 return 0;
 }
 
+int
+virFileGetHugepageSize(const char *path,
+   unsigned long long *size)
+{
+int ret = -1;
+struct statfs fs;
+
+if (statfs(path, &fs) < 0) {
+virReportSystemError(errno,
+ _("cannot determine filesystem for '%s'"),
+ path);
+goto cleanup;
+}
+
+if (fs.f_type != HUGETLBFS_MAGIC) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("not a hugetlbfs mount: '%s'"),
+   path);
+goto cleanup;
+}
+
+*size = fs.f_bsize / 1024; /* we are storing size in KiB */
+ret = 0;
+ cleanup:
+return ret;
+}
+
 #else /* ! HAVE_STATFS */
 
 int virFileIsSharedFSType(const char *path ATTRIBUTE_UNUSED,
@@ -2923,6 +2953,17 @@ int virFileIsSharedFSType(const char *path 
ATTRIBUTE_UNUSED,
 return 0;
 }
 
+int
+virFileGetHugepageSize(const char *path,
+   unsigned long long *size)
+{
+/* XXX implement me :-) */
+VIR_WARN("Trying to get huge page size on %s is not implemented yet.",
+ path);
+*size = 2048; /* Pure guess */
+return 0;
+}
+
 #endif /* HAVE_STATFS */
 
 int virFileIsSharedFS(const char *path)
@@ -2935,3 +2976,117 @@ int virFileIsSharedFS(const char *path)
  VIR_FILE_SHFS_SMB |
  VIR_FILE_SHFS_CIFS);
 }
+
+
+#if defined __linux__ && defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+
+# define PROC_MEMINFO "/proc/meminfo"
+# define HUGEPAGESIZE_STR "Hugepagesize:"
+
+static int
+virFileGetDefaultHugepageSize(unsigned long long *size)
+{
+int ret = -1;
+char *meminfo, *c, *n, *unit;
+
+if (virFileReadAll(PROC_MEMINFO, 4096, &meminfo) < 0)
+goto cleanup;
+
+if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to parse %s"),
+   PROC_MEMINFO);
+goto cleanup;
+}
+c += strlen(HUGEPAGESIZE_STR);
+
+if ((n = strchr(c, '\n'))) {
+/* Cut off the rest of the meminfo file */
+*n = '\0';
+}
+
+if (virStrToLong_ull(c, &unit, 10, size) < 0 || STRNEQ(unit, " kB")) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to parse %s %s"),
+   HUGEPAGESIZE_STR, c);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(meminfo);
+return ret;
+}
+
+# define PROC_MOUNTS "/proc/mounts"
+
+int
+virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
+ size_t *ret_nfs)
+{
+int ret = -1;
+FILE *f = NULL;
+struct mntent mb;
+char mntbuf[1024];
+virHugeTLBFSPtr fs = NULL;
+size_t nfs = 0;
+unsigned long long default_hugepagesz;
+
+if (virFileGetDefaultHugepageSize(&default_hugepagesz) < 0)
+goto cleanup;
+
+if (!(f = setmntent(PROC_MOUNTS, "r"))) {
+virReportSystemError(errno,
+ _("Unable to open %s"),
+ PROC_MOUNTS);
+goto cleanup;
+}
+
+while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) {
+virHugeTLBFSPtr tmp;
+
+if (STRNEQ(mb.mnt_type, "hugetlbfs"))
+continue;
+
+if (VIR_REALLOC_N(fs, nfs + 1) < 0)
+goto cleanup;
+
+tmp = &fs[nfs];
+nfs++;
+
+if (VIR_STRDUP(tmp->mnt_dir, mb.mnt_dir) < 0)
+goto cleanup;
+
+if (virFileGetHugepageSize(tmp->mnt_dir, &tmp->size) < 0)
+goto cleanup;
+
+tmp->deflt = tmp->size == default_hugepagesz;
+}
+
+*ret_fs = fs;
+*ret_nfs = nfs;
+fs = NULL;
+nfs = 0;
+ret = 0;
+
+ cleanup:
+endmntent(f);
+while (nfs)
+VIR_FREE(fs[--nfs].mnt_dir);
+VIR_FREE(fs);
+return r

[libvirt] [PATCH v1 0/7] Hugepages wrt to NUMA

2014-07-17 Thread Michal Privoznik
Up to now, domains are either backed by an arbitrary huge page but without any
NUMA awareness. This is suboptimal and I'm trying to fix it.

Michal Privoznik (7):
  configure: Check for statfs
  Introduce virFileFindHugeTLBFS
  qemu: Utilize virFileFindHugeTLBFS
  virbitmap: Introduce virBitmapDoesIntersect
  domain: Introduce ./hugepages/page/[@size,@unit,@nodeset]
  qemu: Implement ./hugepages/page/[@size,@unit,@nodeset]
  tests: Some testing of hugepages mapping

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

-- 
1.8.5.5

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


Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events

2014-07-17 Thread Nehal J Wani
> My concerns are whether this will work in the case you already used the
> leases helper as the patch is adding a few fields to the stored format.

In this patch, I am only adding server-duid as an extra JSON object. I
am not adding more fields to an existing JSON object. The old leases
file will certainly work, even if server-duid is not present.
Server-DUID will get added in any event which will get triggered from
the ipv6 interface. But all of this will happen assuming dnsmasq and
libvirt have been restarted.

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


Re: [libvirt] [PATCH 1/2] virsh vol-upload/download disallow negative offset

2014-07-17 Thread Peter Krempa
On 07/17/14 01:35, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1087104
> 
> Commit id 'c6212539' explicitly allowed a negative value to be used for
> offset and length as a shorthand for the largest value after commit id
> 'f18c02ec' modified virStrToLong_ui() to essentially disallow a negative
> value.
> 
> However, allowing a negative value for offset ONLY worked if the negative
> value was -1 since the eventual lseek() does allow a -1 to mean the end
> of the file.  Providing other negative values resulted in errors such as:
> 
> $ virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw \
>   --offset -2 --length -1000
> error: cannot download from volume qcow3-vol2
> error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: 
> Invalid argument
> 
> $
> 
> Thus, it seems unreasonable to expect or allow a negative value for offset
> since the only benefit is to lseek() to the end of the file and then only
> take advantage of how the OS would handle such a seek. For the purposes of
> upload or download of volume data, that seems to be a no-op.  Therefore,
> disallow a negative value for offset.
> 
> Additionally, modify the man page for vol-upload and vol-download to provide
> more details regarding the valid values for both offset and length.
> 
> Signed-off-by: John Ferlan 
> ---
>  tools/virsh-volume.c |  6 +++---
>  tools/virsh.pod  | 10 --
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 

ACK, although I'd prefer Eric stating his opinion on this too.

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 2/2] virsh: Document bandwidth maximum more clearly

2014-07-17 Thread Peter Krempa
On 07/17/14 01:35, John Ferlan wrote:
> Commit id '0e2d7305' modified the code to allow a negative value to be
> supplied for the bandwidth argument of the various block virsh commands

Technically that commit didn't allow it. It was allowed before that
commit kept it in that same state.

> and the migrate-setspeed; however, it failed to update the man page to
> describe the "feature" whereby a very large value could be interpreted
> by the hypervisor to mean maximum value allowed. Although initially
> designed to handle a -1 value, the reality is just about any negative
> value could be provided and essentially perform the same feature.
> 
> Signed-off-by: John Ferlan 
> ---
>  tools/virsh.pod | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)

ACK, I like the wording where you specify that the hypervisor may reject
that.

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] leaseshelper: add enhancements to support all events

2014-07-17 Thread Peter Krempa
On 07/16/14 17:31, Nehal J Wani wrote:
>>>  cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
>>>  virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
>>> -virCommandAddArgFormat(cmd, "--dhcp-script=%s", leaseshelper_path);
>>> +virCommandAddArgFormat(cmd, "--dhcp-script=%s", 
>>> pseudo_leaseshelper_path);
>>> +virCommandAddArgFormat(cmd, "--leasefile-ro");
>>
>> Does dnsmasq pass through the rest of the environment we pass to it at
>> this point? The leaseshelper extracts quite a lot of stuff from the
>> environment variables that are passed by dnsmasq. In case it does we
>> could use an env variable to pass the interface name instead of linking
>> the helper and using different names.
> 
> If I use the following line of code in the function
> networkBuildDhcpDaemonCommandLine ...
> setenv("VIR_BRIDGE_NAME", network->def->bridge, 1);
> .. then yes, the helper program invoked by dnsmasq does get to see
> this variable. (It does so in Fedora20). (Looks like I acted in haste,
> should've waited for more replies on RFC, sigh)

I wanted to reply to that but I was a bit busy and didn't manage to do
it soon enough.

We have wrappers for running a separate command with a env variable
virCommandAddEnv*... you can use easily for this.


> 
> So, in the next version that I will send, should I first make a
> wrapper by the name virSetEnvBlockSUID for setenv, just like we have
> virGetEnvBlockSUID for getenv?

No, please use the API mentioned above

> 
>> A second issue that comes into my mind is compatibility with already
>> existing files. Does this work when you already have a lease file? (I
>> didn't try it, I'm just asking).
> 
> If we use the --leasefile-ro option, then although this method will
> work, but no, the lease file will *not* be read/written at all. So
> even if an old one exists, its of no use.
> But then again, the use of --leasefile-ro is mandatory, so that all
> events are captured by our helper program. For example, renew of a
> lease.

My concerns are whether this will work in the case you already used the
leases helper as the patch is adding a few fields to the stored format.

> 
> 
> Regards,
> Nehal J Wani
> 

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] test: metadata: Improve test coverate

2014-07-17 Thread Peter Krempa
On 07/17/14 17:03, Eric Blake wrote:
> On 07/17/2014 08:32 AM, Peter Krempa wrote:
> 
> s/coverate/coverage/ in the title
> 
>> Test also the TITLE and DESCRIPTION metadata types.
>> ---
>>  tests/metadatatest.c | 64 
>> 
>>  1 file changed, 64 insertions(+)
>>
> 
> ACK - more testing is always good
> 

Fixed && 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] spec: Consolidate with_qemu* definitions

2014-07-17 Thread Eric Blake
On 07/17/2014 08:22 AM, Jiri Denemark wrote:
> Decisions whether qemu driver and libvirt-daemon-{qemu,kvm} packages
> should be built on various OS/arch combinations were scattered around
> the spec file. Let's make it easier to see where qemu driver is going to
> be built.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  libvirt.spec.in | 52 +---
>  1 file changed, 25 insertions(+), 27 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 9c7b241..472fa4b 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -54,15 +54,27 @@
>  %define with_vbox  0%{!?_without_vbox:%{server_drivers}}
>  
>  %define with_qemu_tcg  %{with_qemu}
> -# Change if we ever provide qemu-kvm binaries on non-x86 hosts
> -%if 0%{?fedora} >= 18
> +
> +%define qemu_kvm_arches %{ix86} x86_64
> +
> +%if 0%{?fedora}
> +%if 0%{?fedora} < 16
> +# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
> +%ifarch ppc64
> +%define with_qemu_tcg 0
> +%endif
> +%endif

This may be removed if we come to a conclusion in the other thread about
how much we want to support older Fedora.  But we can clean it up later,
this patch is just moving it.

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] test: metadata: Improve test coverate

2014-07-17 Thread Eric Blake
On 07/17/2014 08:32 AM, Peter Krempa wrote:

s/coverate/coverage/ in the title

> Test also the TITLE and DESCRIPTION metadata types.
> ---
>  tests/metadatatest.c | 64 
> 
>  1 file changed, 64 insertions(+)
> 

ACK - more testing is always good

-- 
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] test: metadata: Improve test coverate

2014-07-17 Thread Peter Krempa
Test also the TITLE and DESCRIPTION metadata types.
---
 tests/metadatatest.c | 64 
 1 file changed, 64 insertions(+)

diff --git a/tests/metadatatest.c b/tests/metadatatest.c
index 91fc944..a8d8f10 100644
--- a/tests/metadatatest.c
+++ b/tests/metadatatest.c
@@ -167,6 +167,10 @@ verifyMetadata(virDomainPtr dom,
 struct metadataTest {
 virConnectPtr conn;
 virDomainPtr dom;
+
+const char *data;
+int type;
+bool fail;
 };


@@ -216,6 +220,52 @@ testEraseMetadata(const void *data)
 }

 static int
+testTextMetadata(const void *data)
+{
+const struct metadataTest *test = data;
+char *actual = NULL;
+int ret = -1;
+
+if (virDomainSetMetadata(test->dom, test->type, test->data, NULL, NULL, 0) 
< 0) {
+if (test->fail)
+return 0;
+return -1;
+}
+
+actual = virDomainGetMetadata(test->dom, test->type, NULL, 0);
+
+if (STRNEQ_NULLABLE(test->data, actual)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "expected metadata doesn't match actual: "
+   "expected:'%s'\ngot: '%s'",
+   NULLSTR(test->data), NULLSTR(actual));
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(actual);
+
+return ret;
+}
+
+#define TEST_TEXT_METADATA(INDEX, TYPE, DATA, FAIL) \
+do {\
+test.type = VIR_DOMAIN_METADATA_ ## TYPE;   \
+test.data = DATA;   \
+test.fail = FAIL;   \
+\
+if (virtTestRun("text metadata: " #TYPE " " INDEX " ",  \
+testTextMetadata, &test) < 0)   \
+ret = EXIT_FAILURE; \
+} while (0)
+
+#define TEST_TITLE(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, false)
+#define TEST_TITLE_FAIL(INDEX, DATA) TEST_TEXT_METADATA(INDEX, TITLE, DATA, 
true)
+#define TEST_DESCR(INDEX, DATA) TEST_TEXT_METADATA(INDEX, DESCRIPTION, DATA, 
false)
+
+static int
 mymain(void)
 {
 struct metadataTest test;
@@ -238,6 +288,20 @@ mymain(void)
 if (virtTestRun("Erase metadata ", testEraseMetadata, &test) < 0)
 ret = EXIT_FAILURE;

+TEST_TITLE("1", "qwert");
+TEST_TITLE("2", NULL);
+TEST_TITLE("3", "blah");
+TEST_TITLE_FAIL("4", "qwe\nrt");
+TEST_TITLE("5", "");
+TEST_TITLE_FAIL("6", "qwert\n");
+TEST_TITLE_FAIL("7", "\n");
+
+TEST_DESCR("1", "qwert\nqwert");
+TEST_DESCR("2", NULL);
+TEST_DESCR("3", "qwert");
+TEST_DESCR("4", "\n");
+TEST_DESCR("5", "");
+
 virDomainFree(test.dom);
 virConnectClose(test.conn);

-- 
2.0.0

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


Re: [libvirt] virsh capabilities vs. domcapabilities

2014-07-17 Thread Eric Blake
On 07/17/2014 03:05 AM, Michal Privoznik wrote:

>> Furthermore, I'm trying to figure out how to advertise whether a given
>> domain will support active commit (and similarly, Peter's patches for
>> relative backing name preservation).  Advertising the feature just
>> through 'virsh capabilities' is insufficient, because that only covers
>> the default binary; so it seems like the sort of thing that should also
>> be in 'virsh domcapabilities'.
> 
> That depends on how's active commit accepted by libvirt. IIUC it's a
> flag to an API. (Okay, you got me there, I'm not paying much attention
> to snapshot work).

You use a flag to trigger it for now (although there is a proposal to
eventually allow active commit without a flag); but the main point is
that people want to know up front if the flag even stands a chance of
working, which depends on the abilities of the qemu binary.

> 
> The best solution would be to introduce another section, where supported
> flags to APIs would be enumerated (same way that attribute values are).
> But this is sooo much more work than in attribute part (esp. without
> introspection) that the resulting code would be unmaintainable.
> 

A list of supported flags may be too hard, compared to just a list of
feature names.

>> I'm also finding 'virsh domcapabilities' a bit hard to use - even though
>> it allows all its arguments to be optional at the RPC level, the qemu
>> implementation isn't so happy:
>>
>> # tools/virsh domcapabilities
>> error: failed to get emulator capabilities
>> error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL
>>
>> but how am I supposed to know what --virttype strings are valid?
> 
> By reading the documentation :P
> From the virsh manpage:
> 
> The virttype option specifies the virtualization type used. The value to
> be used is either from the 'type' attribute of the  top level
> element from the domain XML or the 'type' attribute found within each
>  element from the virsh capabilities output.

You know, I can think of a couple of UI additions that would make this
command really cool:

- virsh domcapabilities --domain $dom
uses the same name, uuid, or id as all other domain lookups, then calls
dumpxml on that domain, scrapes the  XML, and uses the
parameters it found to pass to the virConnectGetDomainCapabilities call

- virsh domcapabilities --xml $file
takes $file which contains either  or  XML (no ambiguity,
because the top level element says which style), scrapes it for the
right parameters, and passes those parameters (or NULL for missing
parameters) to virConnectGetDomainCapabilities

Remember, it is possible to define a  with the user input XML
not listing an , where a later dumpxml will show the default
emulator that got chosen.  So the same idea applies to the virsh command
- if the user supplies a  XML file without an explicit
, then we use NULL for that parameter to
virConnectGetDomainCapabilities, and we should get results for the
default emulator that will be used to run that domain.

Also, by allowing  XML (as a subset of overall 'virsh
capabilities') as the input, it is possible to easily transition from
the old API (tell me what emulator(s) will be used) to the new (tell me
what features/drivers I can use with that emulator).

> 
> I know virsh user is user unfriendly, but I think this could be solved
> by wise auto completion (if I find another student to complete it). Or
> if you have any idea meanwhile ...

There's a pending patch series (now several months old) that improves
some auto completion; I need to find time to review and revive it.


>>
>> /guest/arch/emulator vs. /domainCapabilities/path - why 3 levels vs. 2,
>> and different naming?
> 
> This is something provided by callee, so in fact it has no additional
> value.

Not true.  If you take my argument above about allowing NULL emulator to
mean "tell me the default emulator and its capabilities", then this
output parameter is essential in that case.

> 
>>
>> /guest/arch/machine@maxCpus vs. /domainCapabilities/vcpu@max - why 3
>> levels vs. 2, with different naming?
> 
> Because we may want to extend  fom domaincaps in the future.

Okay, so we are intentionally breaking design from the old XML, and
clients cannot share XPath code between the old style and new style.  I
can live with that; although it might be worth some documentation on the
old API how to find the same information in the new.  Maybe as simple as
just documenting all the mappings that I already identified in this
mail.  I know - patches welcome :)

-- 
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] spec: Consolidate with_qemu* definitions

2014-07-17 Thread Jiri Denemark
Decisions whether qemu driver and libvirt-daemon-{qemu,kvm} packages
should be built on various OS/arch combinations were scattered around
the spec file. Let's make it easier to see where qemu driver is going to
be built.

Signed-off-by: Jiri Denemark 
---
 libvirt.spec.in | 52 +---
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9c7b241..472fa4b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -54,15 +54,27 @@
 %define with_vbox  0%{!?_without_vbox:%{server_drivers}}
 
 %define with_qemu_tcg  %{with_qemu}
-# Change if we ever provide qemu-kvm binaries on non-x86 hosts
-%if 0%{?fedora} >= 18
+
+%define qemu_kvm_arches %{ix86} x86_64
+
+%if 0%{?fedora}
+%if 0%{?fedora} < 16
+# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
+%ifarch ppc64
+%define with_qemu_tcg 0
+%endif
+%endif
+%if 0%{?fedora} >= 18
+%define qemu_kvm_arches %{ix86} x86_64 ppc64 s390x
+%endif
 %if 0%{?fedora} >= 20
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x %{arm}
-%else
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x
+%define qemu_kvm_arches %{ix86} x86_64 ppc64 s390x %{arm}
 %endif
-%else
-%define qemu_kvm_arches%{ix86} x86_64
+%endif
+
+%if 0%{?rhel}
+%define with_qemu_tcg 0
+%define qemu_kvm_arches x86_64
 %endif
 
 %ifarch %{qemu_kvm_arches}
@@ -71,6 +83,10 @@
 %define with_qemu_kvm  0
 %endif
 
+%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
+%define with_qemu 0
+%endif
+
 # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
 %define with_openvz0%{!?_without_openvz:1}
 %define with_vmware0%{!?_without_vmware:1}
@@ -191,34 +207,16 @@
 %define with_firewalld 1
 %endif
 
-# RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC
+# RHEL-5 is too old for LXC
 %if 0%{?rhel} == 5
-%define with_qemu_tcg 0
-%ifnarch x86_64
-%define with_qemu 0
-%define with_qemu_kvm 0
-%endif
 %define with_lxc 0
 %endif
 
-# RHEL-6 has restricted QEMU to x86_64 only, stopped including Xen
-# on all archs. Other archs all have LXC available though
+# RHEL-6 stopped including Xen on all archs.
 %if 0%{?rhel} >= 6
-%define with_qemu_tcg 0
-%ifnarch x86_64
-%define with_qemu 0
-%define with_qemu_kvm 0
-%endif
 %define with_xen 0
 %endif
 
-# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
-%if 0%{?fedora} && 0%{?fedora} < 16
-%ifarch ppc64
-%define with_qemu 0
-%endif
-%endif
-
 # Fedora doesn't have new enough Xen for libxl until F18
 %if 0%{?fedora} && 0%{?fedora} < 18
 %define with_libxl 0
-- 
2.0.0

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


Re: [libvirt] [PATCH] Log an error when we fail to set the COW attribute

2014-07-17 Thread Ján Tomko
On 07/17/2014 01:54 PM, John Ferlan wrote:
> 
> 
> On 07/17/2014 06:22 AM, Ján Tomko wrote:
>> Coverity complains about the return value of ioctl not being checked.
>>
>> Even though we carry on when this fails (just like qemu-img does),
>> we can log an error.
>> ---
>>  src/storage/storage_backend.c | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>> index 5e7aa3c..b8b89ca 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>> @@ -464,9 +464,14 @@ virStorageBackendCreateRaw(virConnectPtr conn 
>> ATTRIBUTE_UNUSED,
>>   * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
>>   * failure of this operation should not block the left work.
>>   */
>> -if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
>> +if (ioctl(fd, FS_IOC_GETFLAGS, &attr) < 0) {
>> +virReportSystemError(errno, "%s", _("Failed to get fs flags"));
>> +} else {
>>  attr |= FS_NOCOW_FL;
>> -ioctl(fd, FS_IOC_SETFLAGS, &attr);
>> +if (ioctl(fd, FS_IOC_SETFLAGS, &attr) < 0) {
>> +virReportSystemError(errno, "%s",
>> + _("Failed to set NOCOW flag"));
>> +}
>>  }
>>  #endif
>>  }
>>
> 
> Looks like you were already looking at the failure when my Coverity scan
> ran... Should have checked if there already was a patch before sending
> my response in Chunyan Liu's original series.
> 
> In any case, seems better to me to at least log the error whether or not
> the code wants to "block the left work" (any chance to clean that
> comment up a bit :-) - hopefully there's not too much "right work" left
> either).
> 
> ACK,
> 

I've changed "left work" to "volume creation" and pushed the patch.

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] examples: Introduce domtop

2014-07-17 Thread Ján Tomko
On 07/16/2014 03:53 PM, Michal Privoznik wrote:
> There's this question on the list that is asked over and over again.
> How do I get {cpu, memory, ...} usage in percentage? Or its modified
> version: How do I plot nice graphs like virt-manager does?
> 
> It would be nice if we have an example to inspire people. And that's
> what domtop should do. Yes, it could be written in different ways, but
> I've chosen this one as I think it show explicitly what users need to
> implement in order to imitate virt-manager's graphing.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  .gitignore  |   1 +
>  Makefile.am |   2 +-
>  cfg.mk  |   2 +-
>  configure.ac|   1 +
>  examples/domtop/Makefile.am |  27 +++
>  examples/domtop/domtop.c| 388 
> 
>  libvirt.spec.in |   2 +-
>  7 files changed, 420 insertions(+), 3 deletions(-)
>  create mode 100644 examples/domtop/Makefile.am
>  create mode 100644 examples/domtop/domtop.c

> +
> +static void
> +print_cpu_usage(const char *dom_name,
> +size_t cpu,
> +size_t ncpus,
> +unsigned long long then,
> +virTypedParameterPtr then_params,
> +size_t then_nparams,
> +unsigned long long now,
> +virTypedParameterPtr now_params,
> +size_t now_nparams)
> +{
> +size_t i, j, k;
> +size_t nparams = now_nparams;
> +
> +if (then_nparams != now_nparams) {
> +/* this should not happen (TM) */
> +ERROR("parameters counts don't match");
> +return;
> +}
> +
> +for (i = 0; i < ncpus; i++) {
> +size_t pos;
> +double usage;
> +
> +/* check if the vCPU is in the maps */
> +if (now_params[i * nparams].type == 0 ||
> +then_params[i * then_nparams].type == 0)
> +continue;
> +
> +for (j = 0; j < nparams; j++) {
> +pos = i * nparams + j;
> +if (STREQ(then_params[pos].field, VIR_DOMAIN_CPU_STATS_CPUTIME) 
> ||
> +STREQ(then_params[pos].field, VIR_DOMAIN_CPU_STATS_VCPUTIME))
> +break;
> +}
> +
> +if (j == nparams) {
> +ERROR("unable to find %s", VIR_DOMAIN_CPU_STATS_CPUTIME);
> +return;
> +}
> +
> +DEBUG("now_params=%llu then_params=%llu now=%llu then=%llu",
> +  now_params[pos].value.ul, then_params[pos].value.ul, now, 
> then);
> +
> +/* @now_params and @then_params are in nanoseconds, @now and @then 
> are
> + * in microseconds. In ideal world, we would translate them both into
> + * the same scale, divide one by another and multiply by factor of 
> 100
> + * to get percentage. However, the count of floating point operations
> + * performed has a bad affect on the precision, so instead of 
> dividing

s/affect/effect/

> + * @now_params and @then_params by 1000 and then multiplying again by
> + * 100, we divide only once by 10 and get the same result. */
> +usage = (now_params[pos].value.ul - then_params[pos].value.ul) /
> +(now - then) / 10;
> +
> +printf("CPU%zu: %.2lf\n", cpu + i, usage);

I think printing all the CPUs on one line would look nicer, and it would be
easier to see which numbers are changing.

ACK

Jan



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

Re: [libvirt] [PATCH] Log an error when we fail to set the COW attribute

2014-07-17 Thread John Ferlan


On 07/17/2014 06:22 AM, Ján Tomko wrote:
> Coverity complains about the return value of ioctl not being checked.
> 
> Even though we carry on when this fails (just like qemu-img does),
> we can log an error.
> ---
>  src/storage/storage_backend.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 5e7aa3c..b8b89ca 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -464,9 +464,14 @@ virStorageBackendCreateRaw(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>   * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
>   * failure of this operation should not block the left work.
>   */
> -if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +if (ioctl(fd, FS_IOC_GETFLAGS, &attr) < 0) {
> +virReportSystemError(errno, "%s", _("Failed to get fs flags"));
> +} else {
>  attr |= FS_NOCOW_FL;
> -ioctl(fd, FS_IOC_SETFLAGS, &attr);
> +if (ioctl(fd, FS_IOC_SETFLAGS, &attr) < 0) {
> +virReportSystemError(errno, "%s",
> + _("Failed to set NOCOW flag"));
> +}
>  }
>  #endif
>  }
> 

Looks like you were already looking at the failure when my Coverity scan
ran... Should have checked if there already was a patch before sending
my response in Chunyan Liu's original series.

In any case, seems better to me to at least log the error whether or not
the code wants to "block the left work" (any chance to clean that
comment up a bit :-) - hopefully there's not too much "right work" left
either).

ACK,

John

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


Re: [libvirt] [PATCH V3 1/2] storagevol: add nocow to vol xml

2014-07-17 Thread John Ferlan


Coverity flagged this commit  see below...

On 07/15/2014 04:49 AM, Chunyan Liu wrote:
> Add 'nocow' to storage volume xml so that user can have an option
> to set NOCOW flag to the newly created volume. It's useful on btrfs
> file system to enhance performance.
> 
> Btrfs has low performance when hosting VM images, even more when the guest
> in those VM are also using btrfs as file system. One way to mitigate this
> bad performance is to turn off COW attributes on VM files. Generally, there
> are two ways to turn off COW on btrfs: a) by mounting fs with nodatacow,
> then all newly created files will be NOCOW. b) per file. Add the NOCOW file
> attribute. It could only be done to empty or new files.
> 
> This patch tries the second way, according to 'nocow' option, it could set
> NOCOW flag per file:
> for raw file images, handle 'nocow' in libvirt code; for non-raw file images,
> pass 'nocow=on' option to qemu-img, and let qemu-img to handle that (requires
> qemu-img version >= 2.1).
> 
> Signed-off-by: Chunyan Liu 
> ---
> Changes:
>   * fix typo
> 
>  docs/formatstorage.html.in|  7 +++
>  docs/schemas/storagevol.rng   |  5 +
>  src/conf/storage_conf.c   |  3 +++
>  src/storage/storage_backend.c | 22 ++
>  src/util/virstoragefile.h |  1 +
>  5 files changed, 38 insertions(+)
> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 1cd82b4..8d51402 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -385,6 +385,7 @@
>  
>
>1.1
> +  
>
>  
>
> @@ -424,6 +425,12 @@
>  1.1 is used. If omitted, qemu-img default is used.
>  Since 1.1.0
>
> +  nocow
> +  Turn off COW of the newly created volume. So far, this is only 
> valid
> +for a file image in btrfs file system. It will improve performance 
> when
> +the file image is used in VM. To create non-raw file images, it
> +requires QEMU version since 2.1. Since 
> 1.2.6
> +  
>features
>Format-specific features. Only used for qcow2 now.
>  Valid sub-elements are:
> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
> index 3798476..1b2d4cc 100644
> --- a/docs/schemas/storagevol.rng
> +++ b/docs/schemas/storagevol.rng
> @@ -138,6 +138,11 @@
>
>  
>  
> +  
> +
> +  
> +
> +
>
>  
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index aa29658..f28a314 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1285,6 +1285,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>  virStringFreeList(version);
>  }
>  
> +if (virXPathNode("./target/nocow", ctxt))
> +ret->target.nocow = true;
> +
>  if (options->featureFromString && virXPathNode("./target/features", 
> ctxt)) {
>  if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0)
>  goto error;
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 7b17ca4..5f2dc5d 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -37,6 +37,9 @@
>  #ifdef __linux__
>  # include 
>  # include 
> +# ifndef FS_NOCOW_FL
> +#  define FS_NOCOW_FL 0x0080 /* Do not cow file */
> +# endif
>  #endif
>  
>  #if WITH_SELINUX
> @@ -453,6 +456,21 @@ virStorageBackendCreateRaw(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>  goto cleanup;
>  }
>  
> +if (vol->target.nocow) {
> +#ifdef __linux__
> +int attr;
> +
> +/* Set NOCOW flag. This is an optimisation for btrfs.
> + * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
> + * failure of this operation should not block the left work.
> + */
> +if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +attr |= FS_NOCOW_FL;
> +ioctl(fd, FS_IOC_SETFLAGS, &attr);

  ^^^

Status is not checked here causing complaint about CHECKED_RETURN

John
> +}
> +#endif
> +}
> +
>  if ((ret = createRawFile(fd, vol, inputvol)) < 0)
>  /* createRawFile already reported the exact error. */
>  ret = -1;
> @@ -718,6 +736,7 @@ virStorageBackendCreateQemuImgOpts(char **opts,
> bool preallocate,
> int format,
> const char *compat,
> +   bool nocow,
> virBitmapPtr features)
>  {
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -730,6 +749,8 @@ virStorageBackendCreateQemuImgOpts(char **opts,
>  virBufferAddLit(&buf, "encryption=on,");
>  if

[libvirt] [PATCH] Log an error when we fail to set the COW attribute

2014-07-17 Thread Ján Tomko
Coverity complains about the return value of ioctl not being checked.

Even though we carry on when this fails (just like qemu-img does),
we can log an error.
---
 src/storage/storage_backend.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 5e7aa3c..b8b89ca 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -464,9 +464,14 @@ virStorageBackendCreateRaw(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
  * failure of this operation should not block the left work.
  */
-if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
+if (ioctl(fd, FS_IOC_GETFLAGS, &attr) < 0) {
+virReportSystemError(errno, "%s", _("Failed to get fs flags"));
+} else {
 attr |= FS_NOCOW_FL;
-ioctl(fd, FS_IOC_SETFLAGS, &attr);
+if (ioctl(fd, FS_IOC_SETFLAGS, &attr) < 0) {
+virReportSystemError(errno, "%s",
+ _("Failed to set NOCOW flag"));
+}
 }
 #endif
 }
-- 
1.8.5.5

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


Re: [libvirt] [RFC] require for suggestions on support for ivshmem device

2014-07-17 Thread Martin Kletzander

On Tue, May 20, 2014 at 11:17:32AM +0200, Martin Kletzander wrote:

On Wed, May 14, 2014 at 08:23:21AM +, Wangrui (K) wrote:

Hi,

Libvirt does not support ivshmem(Inter-VM Shared Memory) device recently,
thus, I would like to know if there's any plan to support it in the future?
If not, I would like to contribute a serial of patches to do so.



I came back to this mail right now because I need to have this
implemented.  Is there any progress on your side with this or should I
try hitting this?

[...]

There are two ways to use ivshmem with qemu
(please refer to http://qemu.weilnetz.de/qemu-doc.html#pcsys_005fother_005fdevs 
):
1.Guests map a POSIX shared memory region into the guest as a PCI device
that enables zero-copy communication to the application level of the guests, 
The basic syntax is:

 qemu-system-i386-device ivshmem, size =  [, shm = 
]

2.If desired, interrupts can be sent between guest VMs accessing the same 
shared memory region.
Interrupt support requires using a shared memory server and using a chardev 
socket to connect to it.
An example syntax when using the shared memory server is:

 qemu-system-i386-device ivshmem, size =  [, chardev = 
] [, msi = on]
  [, ioeventfd = on] [, vectors = n] [, role = peer 
| master]
 qemu-system-i386-chardev socket, path = , id = 

The respective xml configuration for the above 2 qemu command lines are shown 
as below:

Example1: automatically attach device with KVM

 
   
 
   
 

NOTE: "size" means ivshmem size in unit MB, "name" means shm name
 "role" is optional, it may be set to "master" or "peer", the default one is 
"master"



What do these roles mean, I mean what's the difference between master
and peer and why is it only used with the chardev?  Does it mean
master can only send interrupts or...?  Just curious.



@Cam (Cc'd) I was wondering about the role= options, so I looked into
the code.  It looks like role=peer just effectively disables
migration.  Did I miss any other difference?

From the libvirt's POV I'd have a few more questions if I may.  How
does the migration work (if there's role=master) WRT other guests
using the same shm?  I found no shm_unlink call in QEMU sources (but
again, I'm not experienced in QEMU's internals), does that mean that
cleanup should be done by libvirt?

Thank you for any info provided.

Martin


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

Re: [libvirt] [PATCH] libxl: Implement basic video device selection

2014-07-17 Thread Stefan Bader
On 16.07.2014 23:05, Jim Fehlig wrote:
> Stefan Bader wrote:
>> being as bad with timely responses. Ok, so how about the following?
>>
>> One note: it could be the STRDUP's are not strictly needed. But
>> to me it felt wrong to have two places refer to the same strings
>> (as MakeVFB copies the struct containing the pointers).
> 
> Agreed.  Without the STRDUP's, seems there is a potential for double
> free when libxl_device_vfb and libxl_domain_config objects are disposed.
> 
>>  If this
>> is not needed, then all changes now in MakeVFB probably can be
>> dropped (except setting the keyboard layout, maybe; which I
>> might miss ;)).
>>
>> -Stefan
>>
>>
>> >From a95db265fa4c1a231e7c2d70baa360c6a0500e3b Mon Sep 17 00:00:00 2001
>> From: Stefan Bader 
>> Date: Thu, 27 Mar 2014 16:01:18 +0100
>> Subject: [PATCH] libxl: Implement basic video device selection
>>
>> This started as an investigation into an issue where libvirt (using the
>> libxl driver) and the Xen host, like an old couple, could not agree on
>> who is responsible for selecting the VNC port to use.
>>
>> Things usually (and a bit surprisingly) did work because, just like that
>> old couple, they had the same idea on what to do by default. However it
>> was possible that this ended up in a big argument.
>>
>> The problem is that display information exists in two different places:
>> in the vfbs list and in the build info. And for launching the device model,
>> only the latter is used. But that never gets initialized from libvirt. So
>> Xen allows the device model to select a default port while libvirt thinks
>> it has told Xen that this is done by libvirt (though the vfbs config).
>>
>> While fixing that, I made a stab at actually evaluating the configuration
>> of the video device. So that it is now possible to at least decide between
>> a Cirrus or standard VGA emulation and to modify the VRAM within certain
>> limits using libvirt.
>>
>> [v2: Check return code of VIR_STRDUP and fix indentation]
>> [v3: Split out VRAM fixup and return error for unsupported video type]
>> [v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
>>   
> 
> [meta-comment]
> libvirt prefers patch version history like this to be below  the '---'
> following your Signed-off-by, so as to not pollute the commit message.

Ah yeah, makes sense. I try to keep it in mind.

> 
>> Signed-off-by: Stefan Bader 
>> ---
>>  src/libxl/libxl_conf.c |   63 
>> ++--
>>  1 file changed, 61 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 8eeaf82..43cabcf 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -1098,10 +1098,21 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>>  libxl_domain_build_info *b_info = &d_config->b_info;
>>  libxl_device_vfb vfb = d_config->vfbs[0];
>>  
>> -if (libxl_defbool_val(vfb.vnc.enable))
>> +if (libxl_defbool_val(vfb.vnc.enable)) {
>>  memcpy(&b_info->u.hvm.vnc, &vfb.vnc, sizeof(libxl_vnc_info));
>> -else if (libxl_defbool_val(vfb.sdl.enable))
>> +if (VIR_STRDUP(b_info->u.hvm.vnc.listen, vfb.vnc.listen) < 0)
>> +goto error;
>> +if (VIR_STRDUP(b_info->u.hvm.vnc.passwd, vfb.vnc.passwd) < 0)
>> +goto error;
>> +} else if (libxl_defbool_val(vfb.sdl.enable)) {
>>  memcpy(&b_info->u.hvm.sdl, &vfb.sdl, sizeof(libxl_sdl_info));
>> +if (VIR_STRDUP(b_info->u.hvm.sdl.display, vfb.sdl.display) < 0)
>> +goto error;
>> +if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, 
>> vfb.sdl.xauthority) < 0)
>> +goto error;
>> +}
>> +if (VIR_STRDUP(b_info->u.hvm.keymap, vfb.keymap) < 0)
>> +goto error;
>>  }
>>  
>>  return 0;
>> @@ -1363,6 +1374,45 @@ libxlMakeCapabilities(libxl_ctx *ctx)
>>  return NULL;
>>  }
>>  
>> +static int
>> +libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config)
>> +{
>> +libxl_domain_build_info *b_info = &d_config->b_info;
>> +
>> +if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_HVM)
>> +return 0;
>> +
>> +/*
>> + * Take the first defined video device (graphics card) to display
>> + * on the first graphics device (display).
>> + * Right now only type and vram info is used and anything beside
>> + * type xen and vga is mapped to cirrus.
>> + */
>> +if (def->nvideos) {
>> +switch (def->videos[0]->type) {
>> +case VIR_DOMAIN_VIDEO_TYPE_VGA:
>> +case VIR_DOMAIN_VIDEO_TYPE_XEN:
>> +b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
>> +break;
>> +case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
>> +b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>> +break;
>> +default:
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>

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

2014-07-17 Thread Chen Hanxiao
Before:
virsh # dominfo chx3
State:  shut off
Max memory: 92160 KiB
Used memory:92160 KiB

After:
virsh # dominfo container1
State:  shut off
Max memory: 92160 KiB
Used memory:0 KiB

Similar to qemu cases.

Signed-off-by: Chen Hanxiao 
---
 src/lxc/lxc_driver.c   | 2 +-
 src/qemu/qemu_driver.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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

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


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

2014-07-17 Thread Daniel P. Berrange
On Thu, Jul 17, 2014 at 11:12:05AM +0200, Michal Privoznik wrote:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 33541d3..7d99435 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16849,17 +16849,17 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
>  char *ret = NULL;
>  virQEMUDriverPtr driver = conn->privateData;
>  virQEMUCapsPtr qemuCaps = NULL;
> -int virttype; /* virDomainVirtType */
> +int virttype = VIR_DOMAIN_VIRT_KVM; /* virDomainVirtType */

Isn't this going to always fail when we run inside a guest which does not
have nested virt? When we know the QEMU binary, we probe to see whether
it can do KVM or not, so could we postpone this decision about virt type
until after we've checked if KVM was available ?

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] virsh capabilities vs. domcapabilities

2014-07-17 Thread Michal Privoznik

On 17.07.2014 11:11, Daniel P. Berrange wrote:

On Thu, Jul 17, 2014 at 11:05:08AM +0200, Michal Privoznik wrote:

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

Would it be nicer to behave the same as 'virsh capabilities' and give
the details of the default binary in this case?


Sure, but in order to get default binary we must know architecture (consider
the case where you have both /usr/bin/qemu-system-x86_64 and
/usr/bin/qemu-system-i686). Although, having only one qemu binary on the
system makes it easy to find the default, doesn't it? Patch on the way.


IMHO it would be preferrable to always default to virArchFromHost()
if arch is none, since that gives a predictable default value, as
opposed to probing emulators which is unpredictable


Yep, that's exactly what I've done in my patch:

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

Michal

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


[libvirt] [PATCH] qemuConnectGetDomainCapabilities: Use wiser defaults

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

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

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

After:

 # virsh domcapabilities
 
   /usr/bin/qemu-system-x86_64
   kvm
   pc-i440fx-2.1
   x86_64
   
 

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 24 ++--
 1 file changed, 6 insertions(+), 18 deletions(-)

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

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


Re: [libvirt] virsh capabilities vs. domcapabilities

2014-07-17 Thread Daniel P. Berrange
On Thu, Jul 17, 2014 at 11:05:08AM +0200, Michal Privoznik wrote:
> ># tools/virsh domcapabilities --virttype kvm
> >error: failed to get emulator capabilities
> >error: invalid argument: at least one of emulatorbin or architecture
> >fields must be present
> >
> >Would it be nicer to behave the same as 'virsh capabilities' and give
> >the details of the default binary in this case?
> 
> Sure, but in order to get default binary we must know architecture (consider
> the case where you have both /usr/bin/qemu-system-x86_64 and
> /usr/bin/qemu-system-i686). Although, having only one qemu binary on the
> system makes it easy to find the default, doesn't it? Patch on the way.

IMHO it would be preferrable to always default to virArchFromHost()
if arch is none, since that gives a predictable default value, as
opposed to probing emulators which is unpredictable

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

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


Re: [libvirt] [PATCH v2 4/8] daemon: support passing FDs from the calling process

2014-07-17 Thread Martin Kletzander

On Thu, Jul 17, 2014 at 09:40:12AM +0100, Daniel P. Berrange wrote:

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

First FD is the RW unix socket to listen on, second one (if
applicable) is the RO unix socket.

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

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

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

+unsigned int cur_fd = STDIN_FILENO + 1;


Shouldn't that be  STDERR not STDIN since the passed FDs start at 3
not 1.



Oh, yes, I tried that with the value "3" and changed it right before
sending, my bad.


+unsigned int nfds = virGetListenFDs();
+


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


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

Re: [libvirt] virsh capabilities vs. domcapabilities

2014-07-17 Thread Michal Privoznik

On 16.07.2014 21:00, Eric Blake wrote:

We have some inconsistencies in the node capabilities (which shows guest
capabilities for some default binaries) and domcapabilities (which shows
guest capabilities for a specified binary).  It might be nicer for
client applications if the two XML components share a common schema and
output layout, so that the same client code can be used to parse either
(sub-tree of) XML, modulo the name of the top-most element containing
the tree.

Furthermore, I'm trying to figure out how to advertise whether a given
domain will support active commit (and similarly, Peter's patches for
relative backing name preservation).  Advertising the feature just
through 'virsh capabilities' is insufficient, because that only covers
the default binary; so it seems like the sort of thing that should also
be in 'virsh domcapabilities'.


That depends on how's active commit accepted by libvirt. IIUC it's a 
flag to an API. (Okay, you got me there, I'm not paying much attention 
to snapshot work).


The best solution would be to introduce another section, where supported 
flags to APIs would be enumerated (same way that attribute values are). 
But this is sooo much more work than in attribute part (esp. without 
introspection) that the resulting code would be unmaintainable.


So my suggestion is to come up with yet another section and put 
arbitrary strings there to represent features like active commit. For 
instance:



  
  
  ...
  

  active-commit
  {some ordinary commit}

  
  





Since virConnectGetDomainCapabilities is brand new to 1.2.7, we still
have time to change its XML.  But I want consensus on whether we need
things to match, or whether we intentionally want people to rely only on
the newer XML format of the new API call (that is, don't bloat 'virsh
capabilities'/virConnectGetCapabilities any further, and learning
whether active commit is supported will have to be done via 'virsh
domcapabilities'/virConnectGetDomainCapabilities).  That is, I'm
wondering if  should use / as
its starting point, rather than completely inventing new XML.

I'm also finding 'virsh domcapabilities' a bit hard to use - even though
it allows all its arguments to be optional at the RPC level, the qemu
implementation isn't so happy:

# tools/virsh domcapabilities
error: failed to get emulator capabilities
error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL

but how am I supposed to know what --virttype strings are valid?


By reading the documentation :P
From the virsh manpage:

The virttype option specifies the virtualization type used. The value to 
be used is either from the 'type' attribute of the  top level 
element from the domain XML or the 'type' attribute found within each 
 element from the virsh capabilities output.


I know virsh user is user unfriendly, but I think this could be solved 
by wise auto completion (if I find another student to complete it). Or 
if you have any idea meanwhile ...




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

Would it be nicer to behave the same as 'virsh capabilities' and give
the details of the default binary in this case?


Sure, but in order to get default binary we must know architecture 
(consider the case where you have both /usr/bin/qemu-system-x86_64 and 
/usr/bin/qemu-system-i686). Although, having only one qemu binary on the 
system makes it easy to find the default, doesn't it? Patch on the way.






Now, for a comparison between the two XML per binary:

'virsh capabilities' gives me this segment:

   
 hvm
 
   64
   /usr/bin/qemu-system-alpha
   clipper
   
   
 
 
   
   
 
   

while 'virsh domcapabilities --emulatorbin /usr/bin/qemu-system-alpha
--virttype kvm' gives this:


   /usr/bin/qemu-system-alpha
   kvm
   clipper
   alpha
   
   
 
   
 disk
 cdrom
 floppy
 lun
   
   
 ide
 fdc
 scsi
 virtio
 usb
   
 
 
   
 subsystem
   
   
 default
 mandatory
 requisite
 optional
   
   
 usb
 pci
 scsi
   
   
   
 
   


  I'm okay that the domcapabilites output is longer, and don't think we
need to backport any of the new stuff to the older API.  But any
information present in the old API should be easily retrieved using the
new API, so that the information isn't lost, and so that a client can
learn the same amount of detail about a non-default binary as they can
about the defaults.

Look at the difference in XPath to get to some of the same information:

/guest/os_type vs. ? - where is os_type in domcapabilities?


nowhere yet.



/guest/arch@name vs. /domainCapabilities/arch - why is one an attribute
and the other an element?

/

Re: [libvirt] virsh capabilities vs. domcapabilities

2014-07-17 Thread Daniel P. Berrange
On Wed, Jul 16, 2014 at 01:00:30PM -0600, Eric Blake wrote:
> Furthermore, I'm trying to figure out how to advertise whether a given
> domain will support active commit (and similarly, Peter's patches for
> relative backing name preservation).  Advertising the feature just
> through 'virsh capabilities' is insufficient, because that only covers
> the default binary; so it seems like the sort of thing that should also
> be in 'virsh domcapabilities'.

My view is that now we have dom capabilities, we shouldn't add anything
more to the old capabilities that is related to guest emulators. The
main motivation is that the old capabilities is not scalable given the
sheer volume of data we need to report per QEMU.

> Since virConnectGetDomainCapabilities is brand new to 1.2.7, we still
> have time to change its XML.  But I want consensus on whether we need
> things to match, or whether we intentionally want people to rely only on
> the newer XML format of the new API call (that is, don't bloat 'virsh
> capabilities'/virConnectGetCapabilities any further, and learning
> whether active commit is supported will have to be done via 'virsh
> domcapabilities'/virConnectGetDomainCapabilities).  That is, I'm
> wondering if  should use / as
> its starting point, rather than completely inventing new XML.

I don't think there's really anything of value in the old capabilities
XML schema that needs preserving as-is.

> I'm also finding 'virsh domcapabilities' a bit hard to use - even though
> it allows all its arguments to be optional at the RPC level, the qemu
> implementation isn't so happy:
> 
> # tools/virsh domcapabilities
> error: failed to get emulator capabilities
> error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL
> 
> but how am I supposed to know what --virttype strings are valid?

Hmm, I'm pretty sure we agreed virt type was going to be optional and
would jsut get a sensible default. So we should fix that.

> 
> # tools/virsh domcapabilities --virttype kvm
> error: failed to get emulator capabilities
> error: invalid argument: at least one of emulatorbin or architecture
> fields must be present
> 
> Would it be nicer to behave the same as 'virsh capabilities' and give
> the details of the default binary in this case?

My original idea was that either emulator or arch would be required.
If we wanted to allow just virtype on its own, I guess we could just
default to the current host arch.

> Now, for a comparison between the two XML per binary:
> 
> 'virsh capabilities' gives me this segment:
> 
>   
> hvm
> 
>   64
>   /usr/bin/qemu-system-alpha
>   clipper
>   
>   
> 
> 
>   
>   
> 
>   
> 
> while 'virsh domcapabilities --emulatorbin /usr/bin/qemu-system-alpha
> --virttype kvm' gives this:
> 
> 
>   /usr/bin/qemu-system-alpha
>   kvm
>   clipper
>   alpha
>   
>   
> 
>   
> disk
> cdrom
> floppy
> lun
>   
>   
> ide
> fdc
> scsi
> virtio
> usb
>   
> 
> 
>   
> subsystem
>   
>   
> default
> mandatory
> requisite
> optional
>   
>   
> usb
> pci
> scsi
>   
>   
>   
> 
>   
> 
> 
>  I'm okay that the domcapabilites output is longer, and don't think we
> need to backport any of the new stuff to the older API.  But any
> information present in the old API should be easily retrieved using the
> new API, so that the information isn't lost, and so that a client can
> learn the same amount of detail about a non-default binary as they can
> about the defaults.
> 
> Look at the difference in XPath to get to some of the same information:
> 
> /guest/os_type vs. ? - where is os_type in domcapabilities?

Yep, that's an oversight really.

> /guest/arch@name vs. /domainCapabilities/arch - why is one an attribute
> and the other an element?
> 
> /guest/arch/wordsize vs. nothing - where is wordsize in domcapabilities?
> 
> /guest/arch/emulator vs. /domainCapabilities/path - why 3 levels vs. 2,
> and different naming?
> 
> /guest/arch/machine@maxCpus vs. /domainCapabilities/vcpu@max - why 3
> levels vs. 2, with different naming?
> 
> /guest/arch/machine vs. /domainCapabilities/machine - why 3 levels vs. 2?

This is just a reflection of the fact that we're doing a different
XML schema. We had the extra '/guest' level in the original XML
because we were reporting on multiple guest types. I see no reason
to make the new XML have this uneccessary extra level of nesting.

> /guest/arch/domain@type vs. /domainCapabilities/domain - why attribute
> of 3 levels vs. element at 2 levels?  And did I expose an error when I
> passed --virrtype kvm, even though qemu-system-alpha is only a qemu
> emulator on my machine (since kvm emulators is only for hardware-assit
> emulation, which is not possible when I'm doing cross architecture)?
> 
> /guest/features vs. ? - where ar

Re: [libvirt] [PATCH v2 4/8] daemon: support passing FDs from the calling process

2014-07-17 Thread Daniel P. Berrange
On Wed, Jul 16, 2014 at 08:29:58PM +0200, Martin Kletzander wrote:
> First FD is the RW unix socket to listen on, second one (if
> applicable) is the RO unix socket.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  daemon/libvirtd.c | 45 +++--
>  1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 4c926b3..d20aeae 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -56,6 +56,7 @@
>  #include "virstring.h"
>  #include "locking/lock_manager.h"
>  #include "viraccessmanager.h"
> +#include "virutil.h"
> 
>  #ifdef WITH_DRIVER_MODULES
>  # include "driver.h"
> @@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv,
>  int unix_sock_ro_mask = 0;
>  int unix_sock_rw_mask = 0;
> 
> +unsigned int cur_fd = STDIN_FILENO + 1;

Shouldn't that be  STDERR not STDIN since the passed FDs start at 3
not 1.

> +unsigned int nfds = virGetListenFDs();
> +

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] Schedule for the next release 1.2.7

2014-07-17 Thread Cedric Bosdonnat
On Thu, 2014-07-17 at 14:51 +0800, Daniel Veillard wrote:
>   Plan is to get the new release on Friday Aug 1st, so
> I think this mean entering freeze at some point on the
> preceeding week-end, I will be travelling then, but I
> should be able to do that at some point saturday 26.
>   We already have 180 commits in since 1.2.6 so deinitely
> makes sense to push this month too,

It would be good if someone could push my 3 patches around LXC keep/drop
capabilities before 1.2.7 freeze. Those were ACKed by Daniel during
1.2.6 freeze.

--
Cedric

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


Re: [libvirt] [PATCHv2 0/2] storage: Don't wipe volumes on remote filesystems with local tools

2014-07-17 Thread Peter Krempa
On 07/16/14 18:54, John Ferlan wrote:
> 
> 
> On 07/11/2014 08:20 AM, Peter Krempa wrote:
>> Version 2 now splits the stuff into separate driver backend funcs.
>>
>> Peter Krempa (2):
>>   storage: wipe: Move helper code into storage backend
>>   storage: Split out volume wiping as separate backend function
>>
>>  src/storage/storage_backend.c | 203 
>> +
>>  src/storage/storage_backend.h |  12 ++
>>  src/storage/storage_backend_disk.c|   1 +
>>  src/storage/storage_backend_fs.c  |   3 +
>>  src/storage/storage_backend_iscsi.c   |   1 +
>>  src/storage/storage_backend_logical.c |   1 +
>>  src/storage/storage_backend_mpath.c   |   1 +
>>  src/storage/storage_backend_scsi.c|   1 +
>>  src/storage/storage_driver.c  | 205 
>> +-
>>  9 files changed, 229 insertions(+), 199 deletions(-)
>>
> 
> ACK both as they seem to fulfill the changes requested from
> http://www.redhat.com/archives/libvir-list/2014-July/msg00556.html
> 

Thanks; Pushed.

> 
> John
> 
> BTW: Since I was looking at a bug (bz 1091866) in this area - this
> caught my attention... Not that these changes fix the problem, but
> perhaps they'll help since as it seems "sparse snapshot" logical volumes
> are "problematic"...
> 

Well with this stuff pushed, you can easily add a wrapper function for
the logical pool that will check whether the volume is sparse and forbid
destroying it before relaying to the wiping function.

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] Syslog "nested job is dangerous" message

2014-07-17 Thread Ján Tomko
On 07/17/2014 08:51 AM, Jiri Denemark wrote:
> On Thu, Jul 17, 2014 at 13:29:52 +1000, Sam Bobroff wrote:
>> On 17/07/14 12:50, Eric Blake wrote:
>>> On 07/16/2014 07:52 PM, Sam Bobroff wrote:
 Hello everyone,
>>>
>>> [Can you configure your mailer to wrap long lines?]
>>
>> [No problem, done.]
>>
 After performing a migration, the syslog often contains several
 messages like this:

 "This thread seems to be the async job owner; entering monitor
 without asking for a nested job is dangerous"
>>>
>>> The sign of a bug that we need to fix.  What version of libvirt are
>>> you using?  We may have already fixed it.
>>
>> I've been testing on 1.2.6, but I will re-test on the latest code from git.
> 
> Hmm, it what were you doing when the message appeared in the log? I
> fixed wrong usage of our job tracking APIs long time ago
> (e4e28220b572cf4c71d07740c24150411f0704a6 in 1.0.3) so I guess there
> must be another place we don't do it correctly.
> 

That commit was essentially reverted by my commit 9846402 when fixing
migration crashes: https://bugzilla.redhat.com/show_bug.cgi?id=1018267

I think the message was harmless during migration as we don't allow other jobs
(except destroy).

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 v3 00/16] Support for per-guest-node binding

2014-07-17 Thread Ján Tomko
On 07/16/2014 04:42 PM, Martin Kletzander wrote:
> v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html
> 
> v3:
>  - Michal's suggestions worked in
>  - rebased on current master
> 
> Martin Kletzander (16):
>   qemu: purely a code movement
>   qemu: remove useless error check
>   conf: purely a code movement
>   conf, schema: add 'id' field for cells
>   numatune: create new module for numatune
>   numatune: unify numatune struct and enum names
>   numatune: Encapsulate numatune configuration in order to unify results
>   conf, schema: add support for memnode elements
>   numatune: add support for per-node memory bindings in private APIs
>   qemu: allow qmp probing for cmdline options without params
>   qemu: memory-backend-ram capability probing
>   qemu: newer -numa parameter capability probing
>   qemu: enable disjoint numa cpu ranges
>   qemu: pass numa node binding preferences to qemu
>   qemu: split out cpuset.mems setting
>   qemu: leave restricting cpuset.mems after initialization
> 
>  docs/formatdomain.html.in  |  32 +-
>  docs/schemas/domaincommon.rng  |  22 +
>  po/POTFILES.in |   1 +
>  src/Makefile.am|   3 +-
>  src/conf/cpu_conf.c|  41 +-
>  src/conf/cpu_conf.h|   3 +-
>  src/conf/domain_conf.c | 203 ++-
>  src/conf/domain_conf.h |  10 +-
>  src/conf/numatune_conf.c   | 595 
> +
>  src/conf/numatune_conf.h   | 108 
>  src/libvirt_private.syms   |  24 +-
>  src/lxc/lxc_cgroup.c   |  20 +-
>  src/lxc/lxc_controller.c   |   5 +-
>  src/lxc/lxc_native.c   |  15 +-
>  src/parallels/parallels_driver.c   |   7 +-
>  src/qemu/qemu_capabilities.c   |  16 +-
>  src/qemu/qemu_capabilities.h   |   2 +
>  src/qemu/qemu_cgroup.c |  52 +-
>  src/qemu/qemu_cgroup.h |   4 +-
>  src/qemu/qemu_command.c|  98 +++-
>  src/qemu/qemu_driver.c |  84 ++-
>  src/qemu/qemu_monitor.c|   6 +-
>  src/qemu/qemu_monitor.h|   3 +-
>  src/qemu/qemu_monitor_json.c   |   8 +-
>  src/qemu/qemu_monitor_json.h   |   3 +-
>  src/qemu/qemu_process.c|  12 +-
>  src/util/virnuma.c |  61 +--
>  src/util/virnuma.h |  28 +-
>  tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |   1 +
>  tests/qemucapabilitiesdata/caps_1.6.50-1.replies   |   5 +
>  tests/qemumonitorjsontest.c|  20 +-
>  .../qemuxml2argv-cpu-numa-disjoint.args|   6 +
>  .../qemuxml2argv-cpu-numa-disjoint.xml |  28 +
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |   6 +-
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |   6 +-
>  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  |  25 +
>  .../qemuxml2argv-numatune-auto-prefer.xml  |  29 +
>  .../qemuxml2argv-numatune-memnode-no-memory.args   |   8 +
>  .../qemuxml2argv-numatune-memnode-no-memory.xml|  30 ++
>  .../qemuxml2argv-numatune-memnode-nocpu.xml|  25 +
>  .../qemuxml2argv-numatune-memnode.args |  11 +
>  .../qemuxml2argv-numatune-memnode.xml  |  33 ++
>  .../qemuxml2argv-numatune-memnodes-problematic.xml |  31 ++
>  tests/qemuxml2argvtest.c   |  12 +
>  .../qemuxml2xmlout-cpu-numa1.xml   |  28 +
>  .../qemuxml2xmlout-cpu-numa2.xml   |  28 +
>  .../qemuxml2xmlout-numatune-auto-prefer.xml|  29 +
>  .../qemuxml2xmlout-numatune-memnode.xml|  33 ++
>  tests/qemuxml2xmltest.c|   8 +
>  49 files changed, 1479 insertions(+), 389 deletions(-)
>  create mode 100644 src/conf/numatune_conf.c
>  create mode 100644 src/conf/numatune_conf.h
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args
>  create mode 100644 tests/qemuxml2argvda