Re: [libvirt] Exposing mem-path in domain XML

[Daniel P. Berrange]

On Wed, Sep 06, 2017 at 01:35:45PM +0200, Michal Privoznik wrote:
> On 09/05/2017 04:07 PM, Daniel P. Berrange wrote:
> > On Tue, Sep 05, 2017 at 03:59:09PM +0200, Michal Privoznik wrote:
> >> On 07/28/2017 10:59 AM, Daniel P. Berrange wrote:
> >>> On Fri, Jul 28, 2017 at 10:45:21AM +0200, Michal Privoznik wrote:
> >>>> On 07/27/2017 03:50 PM, Daniel P. Berrange wrote:
> >>>>> On Thu, Jul 27, 2017 at 02:11:25PM +0200, Michal Privoznik wrote:
> >>>>>> Dear list,
> >>>>>>
> >>>>>> there is the following bug [1] which I'm not quite sure how to grasp. 
> >>>>>> So
> >>>>>> there is this application/infrastructure called Kove [2] that allows 
> >>>>>> you
> >>>>>> to have memory for your application stored on a distant host in network
> >>>>>> and basically fetch needed region on pagefault. Now imagine that
> >>>>>> somebody wants to use it for backing up domain memory. However, the way
> >>>>>> that the tool works is it has some kernel module and then some userland
> >>>>>> binary that is fed with the path of the mmaped file. I don't know all
> >>>>>> the details, but the point is, in order to let users use this we need 
> >>>>>> to
> >>>>>> expose the paths for mem-path for the guest memory. I know we did not
> >>>>>> want to do this in the past, but now it looks like we don't have a way
> >>>>>> around it, do we?
> >>>>>
> >>>>> We don't want to expose the concept of paths in the XML because this is
> >>>>> a linux specific way to configure hugepages / shared memory. So we hide
> >>>>> the particular path used in the internal impl of the QEMU driver, and
> >>>>> or via the qemu.conf global config file. I don't really want to change
> >>>>> that approach, particularly if the only reason is to integrate with a
> >>>>> closed source binary like Kove. 
> >>>>
> >>>> Yep, I agree with that. However, if you read the discussion in the
> >>>> linked bug you'll find that they need to know what file in the
> >>>> memory_backing_dir (from qemu.conf) corresponds to which domain. The
> >>>> reported suggested using UUID based filenames, which I fear is not
> >>>> enough because one can have multiple  -s configured
> >>>> for their domain. But I guess we could go with:
> >>>>
> >>>> ${memory_backing_dir}/${domName}for generic memory
> >>>> ${memory_backing_dir}/${domName}_N  for Nth 
> >>>
> >>> This feels like it is going to lead to hell when you add in memory
> >>> hotplug/unplug, with inevitable races.
> >>>
> >>>> BTW: IIUC they want predictable names because they need to create the
> >>>> files before spawning qemu so that they are picked by qemu instead of
> >>>> using temporary names.
> >>>
> >>> I would like to know why they even need to associate particular memory
> >>> files with particular QEMU processes. eg if they're just exposing a
> >>> new type of tmpfs filesystem from the kernel why does it matter what
> >>> each file is used for.
> >>
> >> This might get you answer:
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1461214#c4
> >>
> >> So the way I understand it is that they will create the files, and
> >> provide us with paths. So luckily, we don't have to make up the paths on
> >> our own.
> > 
> > IOW it is pretending to be tmpfs except it is not behaving like tmpfs.
> > This doesn't really make me any more inclined to support this closed
> > source stuff in libvirt.
> 
> Yeah, that's my feeling too. So, what about the following: let's assume
> they will fix their code so that it is proper tmpfs. Libvirt can then
> behave to it just like it is already doing so for hugetlbfs. For us
> it'll be just yet another type of hugepages. I mean, for hugepages we
> already create /hupages/mount/point/libvirt/$domain per each domain so
> the separation is there (even though this is considered internal impl),
> since it would be a proper tmpfs they can see the pid of qemu which is
> trying to mmap() (and take the name or whatever unique ID they want from
> there).

Yep, we can at least make a reasonable guarantee that all files belonging
to a single QEMU process will always be within the same sub-directory.
This allows the kmod to distinguish 2 files owned by separate VMs, from 2
files owned by the same VM and do what's needed. I don't see why it would
need to care about naming conventions beyond the layout.

> I guess what I'm trying to ask is if it was proper tmpfs, we would be
> okay with it, wouldn't we?

If it is indistinguishable from tmpfs/hugetlbfs from libvirt's POV, we
should be fine -  at most you would need /etc/libvirt/qemu.conf change
to explicitly point at the custom mount point if libvirt doesn't
auto-detect the right one.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH python] Post-release version bump to 3.8.0

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed as trivial

 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 6e5242d..f33ff1a 100755
--- a/setup.py
+++ b/setup.py
@@ -334,7 +334,7 @@ class my_clean(clean):
 _c_modules, _py_modules = get_module_lists()
 
 setup(name = 'libvirt-python',
-  version = '3.7.0',
+  version = '3.8.0',
   url = 'http://www.libvirt.org',
   maintainer = 'Libvirt Maintainers',
   maintainer_email = 'libvir-list@redhat.com',
-- 
2.13.5

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

Re: [libvirt] [PATCH] m4: Disable -Wdisabled-optimization

[Daniel P. Berrange]

On Tue, Sep 05, 2017 at 05:32:03PM +0200, Andrea Bolognani wrote:
> After b4f7793ce269, qemuxml2xmltest has apparently become big enough
> to trigger a compilation error on aarch64:
> 
> CC   qemuxml2xmltest.o
>   qemuxml2xmltest.c: In function 'mymain':
>   qemuxml2xmltest.c:1216:1: error: const/copy propagation disabled: 4361 
> basic blocks and 99285 registers [-Werror=disabled-optimization]
>}
>^
>   qemuxml2xmltest.c:1216:1: error: PRE disabled: 4361 basic blocks and 99285 
> registers [-Werror=disabled-optimization]
>   qemuxml2xmltest.c:1216:1: error: const/copy propagation disabled: 4361 
> basic blocks and 99285 registers [-Werror=disabled-optimization]
>   qemuxml2xmltest.c:1216:1: error: const/copy propagation disabled: 4361 
> basic blocks and 99285 registers [-Werror=disabled-optimization]
> 
> However, as the GCC documentation states, this warning is not really
> caused by issues in our code, so it makes sense to disable it.
> 
> Signed-off-by: Andrea Bolognani <abolo...@redhat.com>
> ---
>  m4/virt-compile-warnings.m4 | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> index e8f135126..f18a08a8f 100644
> --- a/m4/virt-compile-warnings.m4
> +++ b/m4/virt-compile-warnings.m4
> @@ -64,6 +64,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>  # Several conditionals expand the same on both branches
>  # depending on the particular platform/architecture
>  dontwarn="$dontwarn -Wduplicated-branches"
> +# > This warning does not generally indicate that there is anything wrong
> +# > with your code; it merely indicates that GCC's optimizers are unable
> +# > to handle the code effectively.
> +# Source: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
> +dontwarn="$dontwarn -Wdisabled-optimization"
>  
>      # gcc 4.2 treats attribute(format) as an implicit attribute(nonnull),
>  # which triggers spurious warnings for our usage

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] Exposing mem-path in domain XML

[Daniel P. Berrange]

On Tue, Sep 05, 2017 at 03:59:09PM +0200, Michal Privoznik wrote:
> On 07/28/2017 10:59 AM, Daniel P. Berrange wrote:
> > On Fri, Jul 28, 2017 at 10:45:21AM +0200, Michal Privoznik wrote:
> >> On 07/27/2017 03:50 PM, Daniel P. Berrange wrote:
> >>> On Thu, Jul 27, 2017 at 02:11:25PM +0200, Michal Privoznik wrote:
> >>>> Dear list,
> >>>>
> >>>> there is the following bug [1] which I'm not quite sure how to grasp. So
> >>>> there is this application/infrastructure called Kove [2] that allows you
> >>>> to have memory for your application stored on a distant host in network
> >>>> and basically fetch needed region on pagefault. Now imagine that
> >>>> somebody wants to use it for backing up domain memory. However, the way
> >>>> that the tool works is it has some kernel module and then some userland
> >>>> binary that is fed with the path of the mmaped file. I don't know all
> >>>> the details, but the point is, in order to let users use this we need to
> >>>> expose the paths for mem-path for the guest memory. I know we did not
> >>>> want to do this in the past, but now it looks like we don't have a way
> >>>> around it, do we?
> >>>
> >>> We don't want to expose the concept of paths in the XML because this is
> >>> a linux specific way to configure hugepages / shared memory. So we hide
> >>> the particular path used in the internal impl of the QEMU driver, and
> >>> or via the qemu.conf global config file. I don't really want to change
> >>> that approach, particularly if the only reason is to integrate with a
> >>> closed source binary like Kove. 
> >>
> >> Yep, I agree with that. However, if you read the discussion in the
> >> linked bug you'll find that they need to know what file in the
> >> memory_backing_dir (from qemu.conf) corresponds to which domain. The
> >> reported suggested using UUID based filenames, which I fear is not
> >> enough because one can have multiple  -s configured
> >> for their domain. But I guess we could go with:
> >>
> >> ${memory_backing_dir}/${domName}for generic memory
> >> ${memory_backing_dir}/${domName}_N  for Nth 
> > 
> > This feels like it is going to lead to hell when you add in memory
> > hotplug/unplug, with inevitable races.
> > 
> >> BTW: IIUC they want predictable names because they need to create the
> >> files before spawning qemu so that they are picked by qemu instead of
> >> using temporary names.
> > 
> > I would like to know why they even need to associate particular memory
> > files with particular QEMU processes. eg if they're just exposing a
> > new type of tmpfs filesystem from the kernel why does it matter what
> > each file is used for.
> 
> This might get you answer:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1461214#c4
> 
> So the way I understand it is that they will create the files, and
> provide us with paths. So luckily, we don't have to make up the paths on
> our own.

IOW it is pretending to be tmpfs except it is not behaving like tmpfs.
This doesn't really make me any more inclined to support this closed
source stuff in libvirt.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] Yet another RFC for CAT

[Daniel P. Berrange]

On Mon, Sep 04, 2017 at 04:14:00PM +0200, Martin Kletzander wrote:
> * The current design (finally something libvirt-related, right?)
> 
> The discussion ended with a conclusion of the following (with my best
> knowledge, there were so many discussions about so many things that I
> would spend too much time looking up all of them):
> 
> - Users should not need to specify bit masks, such complexity should be
>   abstracted.  We'll use sizes (e.g. 4MB)
> 
> - Multiple vCPUs might need to share the same allocation.
> 
> - Exclusivity of allocations is to be assumed, that is only unoccupied
>   cache should be used for new allocations.
> 
> The last point seems trivial but it's actually very specific condition
> that, if removed, can cause several problems.  If it's hard to grasp the
> last point together with the second one, you're on the right track.  If
> not, then I'll try to make a point for why the last point should be
> removed in 3... 2... 1...
> 
> * Design flaws
> 
> 1) Users cannot specify any allocation that would share only part with
>some other allocation of the domain or the default group.
> 
> 2) It was not specified what to do with the default resource group.
>There might be several ways to approach this, with varying pros and
>cons:
> 
> a) Treat it as any other group.  That is any bit set for this group
>will be excluded from usable bits when creating new allocation
>for a domain.
> 
> - Very predictable behaviour
> 
> - You will not be able to allocate any amount of cache without
>   previous setting for the default group as that will have all
>   the bits set which will make all the cache unusable
> 
> b) Automatically remove the appropriate amount of bits that are
>needed for new domains.
> 
> - No need to do any change to the system settings in order to
>   use this new feature
> 
> - We would have to change system settings, which is generally
>   frowned upon when done "automatically" as a side effect of
>   starting a domain, especially for such scarce resource as
>   cache
> 
> - The change to system settings would not be entirely
>   predictable
> 
> c) Act like it doesn't exist and don't remove its allocations from
>consideration
> 
> - Doesn't really make sense as system processes might be
>   trashing the cache as any VM, moreover when all VM processes
>   without allocations will be based in the default group as
>   well
> 
> 3) There is no way for users to know what the particular settings are
>for any running domain.
> 
> The first point was deemed a corner case.  Fair enough on its own, but
> considering point 2 and its solutions, it is rather difficult for me to
> justify it.  Also, let's say you have domain with 4 vCPUs out of which
> you know 1 might be trashing the cache, but you don't want to restrict
> it completely, but others will utilize it very nicely.  Sensible
> allocations for such domain's vCPUs might be:
> 
>  vCPU  0:   000f
>  vCPUs 1-3: 
> 
> as you want vCPUs 1-3 to utilize even the part of cache that might get
> trashed by vCPU 0.  Or they might share some data (especially
> guest-memory-related).
> 
> The case above is not possible to set up with only per-vcpu(s) scalar
> setting.  And there are more as you might imagine now.  For example how
> do we behave with iothreads and emulator threads?

Ok, I see what you're getting at.  I've actually forgotten what
our current design looks like though :-)

What level of granularity were we allowing within a guest ?
All vCPUs use separate cache regions from each other, or all
vCPUs use a share cached region, but separate from other guests,
or a mix ?

> * My suggestion:
> 
> - Provide an API for querying and changing the allocation of the
>   default resource group.  This would be similar to setting and
>   querying hugepage allocations (see virsh's freepages/allocpages
>   commands).

Reasonable

> - Let users specify the starting position in addition to the size, i.e.
>   not only specifying "size", but also "from".  If "from" is not
>   specified, the whole allocation must be exclusive.  If "from" is
>   specified it will be set without checking for collisions.  The latter
>   needs them to query the system or know what settings are applied
>   (this should be the case all the time), but is better then adding
>   non-specific and/or meaningless exclusivity settings (how do you
>   specify part-exclusivity of the cache as in the example above)

I'm concerned about the idea of not checking 'from' for collisions,
if there's allowed a mix of guests with & within 'from'.

eg consider

 * Initially 24 MB of cache is free, starting at 8MB
 * run guest A   from=8M, size=8M
 * run guest B   size=8M
 => libvirt sets from=16M, so doesn't clash with A
 * stop guest A
 * run guest C   size=8M
 => libvirt sets from=8M, so 

Re: [libvirt] [PATCH] Makefile.nonreentrant: Rebuild against Fedora 26

[Daniel P. Berrange]

On Mon, Sep 04, 2017 at 03:06:16PM +0200, Andrea Bolognani wrote:
> According to the comments in the file and the git history, the
> list of forbidden symbols was originally built against Fedora 9
> in 2009 (!) and pretty much never refreshed afterwards.
> 
> Signed-off-by: Andrea Bolognani <abolo...@redhat.com>
> ---
> I didn't put too much thought into this, but the results looks
> sane enough as we are only *adding* to the list.
> 
>  Makefile.nonreentrant | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH python] Remove unused variables for event callbacks

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-override.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c
index c04ce2e..a3a0508 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -5182,19 +5182,11 @@ libvirt_virConnectDomainEventDeregister(PyObject *self 
ATTRIBUTE_UNUSED,
  * Event Impl
  ***/
 static PyObject *addHandleObj;
-static char *addHandleName;
 static PyObject *updateHandleObj;
-static char *updateHandleName;
 static PyObject *removeHandleObj;
-static char *removeHandleName;
 static PyObject *addTimeoutObj;
-static char *addTimeoutName;
 static PyObject *updateTimeoutObj;
-static char *updateTimeoutName;
 static PyObject *removeTimeoutObj;
-static char *removeTimeoutName;
-
-#define NAME(fn) ( fn ## Name ? fn ## Name : # fn )
 
 static int
 libvirt_virEventAddHandleFunc(int fd,
@@ -5442,12 +5434,6 @@ libvirt_virEventRegisterImpl(PyObject *self 
ATTRIBUTE_UNUSED,
 Py_XDECREF(addTimeoutObj);
 Py_XDECREF(updateTimeoutObj);
 Py_XDECREF(removeTimeoutObj);
-VIR_FREE(addHandleName);
-VIR_FREE(updateHandleName);
-VIR_FREE(removeHandleName);
-VIR_FREE(addTimeoutName);
-VIR_FREE(updateTimeoutName);
-VIR_FREE(removeTimeoutName);
 
 /* Parse and check arguments */
 if (!PyArg_ParseTuple(args, (char *) "OO:virEventRegisterImpl",
@@ -5462,14 +5448,6 @@ libvirt_virEventRegisterImpl(PyObject *self 
ATTRIBUTE_UNUSED,
 !PyCallable_Check(removeTimeoutObj))
 return NULL;
 
-/* Get argument string representations (for error reporting) */
-addHandleName = py_str(addHandleObj);
-updateHandleName = py_str(updateHandleObj);
-removeHandleName = py_str(removeHandleObj);
-addTimeoutName = py_str(addTimeoutObj);
-updateTimeoutName = py_str(updateTimeoutObj);
-removeTimeoutName = py_str(removeTimeoutObj);
-
 /* Inc refs since we're holding on to these objects until
  * the next call (if any) to this function.
  */
-- 
2.13.5

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

[libvirt] [PATCH python] Report an error if registering an event loop twice

[Daniel P. Berrange]

The C library will now ignore an attempt to register an event
loop twice. It is unable to report an error in this case though
due to the C API returning 'void'. To improve this we must
manually report an error at the python level.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-override.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c
index a3a0508..9eba4ed 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -5427,13 +5427,12 @@ static PyObject *
 libvirt_virEventRegisterImpl(PyObject *self ATTRIBUTE_UNUSED,
  PyObject *args)
 {
-/* Unref the previously-registered impl (if any) */
-Py_XDECREF(addHandleObj);
-Py_XDECREF(updateHandleObj);
-Py_XDECREF(removeHandleObj);
-Py_XDECREF(addTimeoutObj);
-Py_XDECREF(updateTimeoutObj);
-Py_XDECREF(removeTimeoutObj);
+if (addHandleObj || updateHandleObj || removeHandleObj ||
+addTimeoutObj || updateTimeoutObj || removeTimeoutObj) {
+PyErr_SetString(PyExc_RuntimeError,
+"Event loop is already registered");
+return NULL;
+}
 
 /* Parse and check arguments */
 if (!PyArg_ParseTuple(args, (char *) "OO:virEventRegisterImpl",
-- 
2.13.5

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

[libvirt] [PATCH] Add libxslt as build requires for mingw RPMs

[Daniel P. Berrange]

The libxslt package is needed since:

  commit 94d2d6429d686c5af95115d09c01f3c6bd5ea7c6
  Author: Daniel P. Berrange <berra...@redhat.com>
  Date:   Wed Jul 26 17:40:44 2017 +0100

docs: make xmllint & xsltproc compulsory

The native RPM had it already, but mingw build was missing it.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed as a build breaker & trivial fix

 mingw-libvirt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 6b0ce0bf5..dc18d055b 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -60,6 +60,7 @@ BuildRequires:  mingw64-dlfcn
 BuildRequires:  pkgconfig
 # Need native version for msgfmt
 BuildRequires:  gettext
+BuildRequires:  libxslt
 BuildRequires:  python
 %if 0%{?fedora} >= 27
 BuildRequires:  perl-interpreter
-- 
2.13.5

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

[libvirt] [PATCH] event: ignore attempts to replace the event loop impl

[Daniel P. Berrange]

Although not previously explicitly documented, the expectation for
the libvirt event loop is that an implementation is registered early
in application startup, before calling any libvirt APIs and then
run forever after. Replacing a previously registered event loop is
not safe & subject to races even if virConnectClose has been called
on open handles, due to delayed deregistration of callbacks during
conenction close.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 src/util/virevent.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/util/virevent.c b/src/util/virevent.c
index e0fd35e41..51d8714df 100644
--- a/src/util/virevent.c
+++ b/src/util/virevent.c
@@ -220,6 +220,13 @@ virEventRemoveTimeout(int timer)
  * existing event loop implementation, then the
  * virEventRegisterDefaultImpl() method can be used to setup
  * the generic libvirt implementation.
+ *
+ * Once registered, the event loop implementation cannot be
+ * changed, and must be run continuously. Note that callbacks
+ * may remain registered for a short time even after calling
+ * virConnectClose on all open connections, so it is not safe
+ * to stop running the event loop immediately after closing
+ * the connection.
  */
 void virEventRegisterImpl(virEventAddHandleFunc addHandle,
   virEventUpdateHandleFunc updateHandle,
@@ -233,6 +240,12 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle,
   addHandle, updateHandle, removeHandle,
   addTimeout, updateTimeout, removeTimeout);
 
+if (addHandleImpl || updateHandleImpl || removeHandleImpl ||
+addTimeoutImpl || updateTimeoutImpl || removeHandleImpl) {
+VIR_WARN("Ignoring attempt to replace registered event loop");
+return;
+}
+
 addHandleImpl = addHandle;
 updateHandleImpl = updateHandle;
 removeHandleImpl = removeHandle;
-- 
2.13.5

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

Re: [libvirt] [PATCH 2/3] libvirtaio: add allow for moving callbacks to other event loop

[Daniel P. Berrange]

On Fri, Sep 01, 2017 at 01:51:17PM +0200, Wojtek Porczyk wrote:
> On Fri, Sep 01, 2017 at 10:08:18AM +0100, Daniel P. Berrange wrote:
> > IIUC, you are trying to make it possible to register multiple event
> > loop impls.  This is *not* supported usage of libvirt. You must
> > call 'virEventRegisterImpl' before opening any connection, and once
> > called you are forbidden to call it again.
> 
> Yes, that's correct. Can't I do it, even after I close all the connections?

That would be racy because some cleanup is liable to happen asynchronously.

> Why then libvirt_virEventRegisterImpl (libvirt-python/libvirt-override.c:5434)
> seems to accomodate for running it second time?

That's bogus code that we should remove - in fact the C library
should simply ignore any subsequent virEventRegisterImpl API
calls after the first one.

> The reason for this is we have separate event loop for each test case, but the
> whole suite runs in a single process. The Impl has to use the new loop for
> each test. Would it be better to just substitute the loop in a long-lived Impl
> instance?

Replacing 'loop' would achieve the same effet i guess, but you must
ensure there are no callbacks still registered by libvirt before doing
that.

Generally the expectation is that you register an event loop and then
run it forever until the process exits.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/3] libvirtaio: add allow for moving callbacks to other event loop

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 09:40:23PM +0200, Wojtek Porczyk wrote:
> The virEvent implementation is tied to a particular loop. When spinning
> another loop, the callbacks have to be moved to another implementation,
> so they will have a chance to be invoked, should they be scheduled. If
> not, file descriptors will be leaking.
> 
> Signed-off-by: Wojtek Porczyk 
> ---
>  libvirtaio.py | 64 
> +++
>  1 file changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/libvirtaio.py b/libvirtaio.py
> index fc868bd..d161cd1 100644
> --- a/libvirtaio.py
> +++ b/libvirtaio.py
> @@ -195,9 +195,10 @@ class FDCallback(Callback):
>  return '<{} iden={} fd={} event={}>'.format(
>  self.__class__.__name__, self.iden, self.descriptor.fd, 
> self.event)
>  
> -def update(self, event):
> +def update(self, event=None):
>  '''Update the callback and fix descriptor's watchers'''
> -self.event = event
> +if event is not None:
> +self.event = event
>  self.descriptor.update()
>  
>  #
> @@ -238,20 +239,21 @@ class TimeoutCallback(Callback):
>  self.cb(self.iden, self.opaque)
>  self.impl.log.debug('timer %r callback ended', self.iden)
>  
> -def update(self, timeout):
> +def update(self, timeout=None):
>  '''Start or the timer, possibly updating timeout'''
> -self.timeout = timeout
> -
> -if self.timeout >= 0 and self._task is None:
> -self.impl.log.debug('timer %r start', self.iden)
> -self._task = ensure_future(self._timer(),
> -loop=self.impl.loop)
> +if timeout is not None:
> +self.timeout = timeout
>  
> -elif self.timeout < 0 and self._task is not None:
> +if self._task is not None:
>  self.impl.log.debug('timer %r stop', self.iden)
>  self._task.cancel()  # pylint: disable=no-member
>  self._task = None
>  
> +if self.timeout >= 0:
> +self.impl.log.debug('timer %r start', self.iden)
> +self._task = ensure_future(self._timer(),
> +loop=self.impl.loop)
> +
>  def close(self):
>  '''Stop the timer and call ff callback'''
>  super(TimeoutCallback, self).close()
> @@ -274,6 +276,7 @@ class virEventAsyncIOImpl(object):
>  self.callbacks = {}
>  self.descriptors = DescriptorDict(self)
>  self.log = logging.getLogger(self.__class__.__name__)
> +self.pending_tasks = set()
>  
>  def register(self):
>  '''Register this instance as event loop implementation'''
> @@ -284,9 +287,30 @@ class virEventAsyncIOImpl(object):
>  self._add_timeout, self._update_timeout, self._remove_timeout)
>  return self
>  
> +def takeover(self, other):
> +'''Take over other implementation, probably registered on another 
> loop
> +
> +:param virEventAsyncIOImpl other: other implementation to be taken 
> over
> +'''
> +self.log.warning('%r taking over %r', self, other)
> +
> +while other.callbacks:
> +iden, callback = other.callbacks.popitem()
> +self.log.debug('  takeover %d %r', iden, callback)
> +assert callback.iden == iden
> +callback.impl = self
> +self.callbacks[iden] = callback
> +
> +if isinstance(callback, FDCallback):
> +fd = callback.descriptor.fd
> +assert callback is other.descriptors[fd].remove_handle(iden)
> +self.descriptors[fd].add_handle(callback)
> +
>  def schedule_ff_callback(self, iden, opaque):
>  '''Schedule a ff callback from one of the handles or timers'''
> -ensure_future(self._ff_callback(iden, opaque), loop=self.loop)
> +fut = ensure_future(self._ff_callback(iden, opaque), loop=self.loop)
> +self.pending_tasks.add(fut)
> +fut.add_done_callback(self.pending_tasks.remove)
>  
>  @asyncio.coroutine
>  def _ff_callback(self, iden, opaque):
> @@ -297,13 +321,19 @@ class virEventAsyncIOImpl(object):
>  self.log.debug('ff_callback(iden=%d, opaque=...)', iden)
>  return libvirt.virEventInvokeFreeCallback(opaque)
>  
> +@asyncio.coroutine
> +def drain(self):
> +self.log.debug('drain()')
> +if self.pending_tasks:
> +yield from asyncio.wait(self.pending_tasks, loop=self.loop)
> +
>  def is_idle(self):
>  '''Returns False if there are leftovers from a connection
>  
>  Those may happen if there are sematical problems while closing
>  a connection. For example, not deregistered events before .close().
>  '''
> -return not self.callbacks
> +return not self.callbacks and not self.pending_tasks
>  
>  def _add_handle(self, fd, event, cb, opaque):
>  '''Register a 

Re: [libvirt] [PATCH v6 01/13] qemu: Add QEMU 2.10 x86_64 the generated capabilities

[Daniel P. Berrange]

On Wed, Aug 30, 2017 at 06:46:01PM -0400, John Ferlan wrote:
> For reference, these were generated by updating a local qemu git
> repository to the latest upstream and building in order to generate
> an "x86_64-softmmu/qemu-system-x86_64" image.
> 
> The image was then provided as input:
> 
> tests/qemucapsprobe /path/to/x86_64-softmmu/qemu-system-x86_64 > \
>tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
> 
> The image was then used in a test domain as the emulator and the
> domain started using a "top of tree" build running libvirtd which
> generates an XML file in /var/cache/libvirt/qemu/capabilities/.
> I search the cache for 2.10:
> 
> grep 2.10 /var/cache/libvirt/qemu/capabilities/*.xml
> 
> and copied the resulting xml file to
> 
> tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
> 
> scrubbed the file slightly to set qemuctime, selfctime, and
> selfvers to 0 and change the package to add a "-dirty".
> 
> Signed-off-by: John Ferlan 
> ---
> 
>  This is new from v5 - since VxHS was added to qemu 2.10 and we
>  don't have qemu 2.10 definitions yet, in order to properly add
>  the capabilities checks, we need the environment set up.
> 
>  .../caps_2.10.0.x86_64.replies | 17994 
> +++
>  tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   791 +
>  tests/qemucapabilitiestest.c   | 1 +
>  3 files changed, 18786 insertions(+)
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
>  create mode 100644 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
> 
> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies 
> b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
> new file mode 100644
> index 000..08a33ab
> --- /dev/null
> +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies
> @@ -0,0 +1,17994 @@
> +{
> +  "QMP": {
> +"version": {
> +  "qemu": {
> +"micro": 94,
> +"minor": 9,
> +"major": 2
> +  },
> +  "package": " (v2.10.0-rc4-dirty)"

QEMU 2.10.0 is now released, so please rebase to the release tag instead of
the -rc4.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] qemu: handle -1 for pid in qemuDomainGetMachineName

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 12:01:44PM +0300, Nikolay Shirokovskiy wrote:
> We call qemuDomainGetMachineName on domain start. On first
> start (after daemon start) pid is 0 and virSystemdGetMachineNameByPID
> don't get called. But after domain shutting down pid became -1 so
> on next start virSystemdGetMachineNameByPID is called and returned an error.
> Error is ignored so it is not critical. But at least on my system
> (systemd-219 with extra patches) systemd-machined is crashed on
> this request.
> 
> This behaviour is triggered by eaf2c9f89.
> ---
>  src/qemu/qemu_domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e580a5b..fe44d36 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9696,7 +9696,7 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
>  virQEMUDriverPtr driver = priv->driver;
>  char *ret = NULL;
>  
> -if (vm->pid) {
> +if (vm->pid > 0) {
>  ret = virSystemdGetMachineNameByPID(vm->pid);
>  if (!ret)
>  virResetLastError();

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>


Worth pushing for this release too IMHO

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] Fix TLS test suites with gnutls 3.6.0

[Daniel P. Berrange]

With gnutls 3.6.0, SHA1 is no longer accepted for certificate
signatures. We must usw SHA256 instead.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/virnettlshelpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
index 531d0b907..b735c4e2f 100644
--- a/tests/virnettlshelpers.c
+++ b/tests/virnettlshelpers.c
@@ -384,7 +384,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
  * If no 'ca' is set then we are self signing
  * the cert. This is done for the root CA certs
  */
-if ((err = gnutls_x509_crt_sign(crt, ca ? ca : crt, privkey)) < 0) {
+if ((err = gnutls_x509_crt_sign2(crt, ca ? ca : crt, privkey, 
GNUTLS_DIG_SHA256, 0)) < 0) {
 VIR_WARN("Failed to sign certificate %s", gnutls_strerror(err));
 abort();
 }
-- 
2.13.5

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

[libvirt] [PATCH] rpc: avoid ssh interpreting malicious hostname as arguments

[Daniel P. Berrange]

Inspired by the recent GIT / Mercurial security flaws
(http://blog.recurity-labs.com/2017-08-10/scm-vulns),
consider someone/something manages to feed libvirt a bogus
URI such as:

  virsh -c qemu+ssh://-oProxyCommand=gnome-calculator/system

In this case, the hosname "-oProxyCommand=gnome-calculator"
will get interpreted as an argument to ssh, not a hostname.
Fortunately, due to the set of args we have following the
hostname, SSH will then interpret our bit of shell script
that runs 'nc' on the remote host as a cipher name, which is
clearly invalid. This makes ssh exit during argv parsing and
so it never tries to run gnome-calculator.

We are lucky this time, but lets be more paranoid, by using
'--' to explicitly tell SSH when it has finished seeing
command line options. This forces it to interpret
"-oProxyCommand=gnome-calculator" as a hostname, and thus
see a fail from hostname lookup.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 src/rpc/virnetsocket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index d228c8a8c..23089afef 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -868,7 +868,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
 if (!netcat)
 netcat = "nc";
 
-virCommandAddArgList(cmd, nodename, "sh", "-c", NULL);
+virCommandAddArgList(cmd, "--", nodename, "sh", "-c", NULL);
 
 virBufferEscapeShell(, netcat);
 if (virBufferCheckError() < 0) {
-- 
2.13.5

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

Re: [libvirt] Entering freeze for libvirt-3.7.0

[Daniel P. Berrange]

On Tue, Aug 29, 2017 at 02:12:56PM +0200, Daniel Veillard wrote:
>   As suggested yesterday (sorry for short notice) I have just tagged the
> rc1 in git and pushed signed tarball and rpms to the usual place:
> 
>   ftp://libvirt.org/libvirt/
> 
>  Things looks fine for me, jenkins on https://ci.centos.org/view/libvirt/
> seems mostly happy (though there is that master rpm weirdness) so it

It seems our test suite for TLS is broken by gnutls 3.6.0 that just
hit fedora rawhide. IIUC, we are generating certs using the "NORMAL"
gnutls priority (which defaults to SHA1 signature alg), but we're
trying to verify with "SYSTEM" priority (which forbids SHA1 signature
alg).


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] New QEMU daemon for persistent reservations

[Daniel P. Berrange]

On Tue, Aug 22, 2017 at 06:27:40PM +0200, Paolo Bonzini wrote:
> Hi all,
> 
> I am adding a new daemon to QEMU, that QEMU can connect to in order to
> issue persistent reservation commands.

Can you elaborate on what this daemon does ?

IIUC, by 'persistent reservation' you are referring to SCSI LUN
reservations ?  If so, the security model needs to involve more
than just policy on the socket that QEMU uses to talk to the
PR daemon.  We need to be able to control what device nodes this
daemon is permitted to access.

For iSCSI backed disks, the daemon might need to use the libiscsi
driver, instead of assuming there is a device node on the local
host too.

Libvirt has a generic lock manager facility and I've previously
though might be able to be extended to acquire SCSI reservations
for disks which are backed by SCSI/iSCSI, but never tried to
work on this.

> The daemon can only issue the commands on file descriptor that QEMU
> already has.  In addition normal users shouldn't have access to the
> daemon's Unix socket in /run, so the daemon is protected against misuse.
> 
> My question is what is the best way to handle the connection to the
> daemon socket.  Currently, the path to the socket is passed to QEMU on
> the command line:
> 
>  -object pr-manager-helper,id=mgr,path=/run/qemu-pr-helper.sock \
>  -drive if=none,id=hd,driver=raw,filename=/dev/sdb,file.pr-manager=mgr \
>  -device scsi-block,drive=hd
> 
> (the new parts are "-object pr-manager-helper" and "file.pr-manager").

I'm not sure if want to have QEMU spawning this daemon itself or not.

On the one hand if QEMU spawns the daemon, then it means the daemon
inherits the SELinux policy of QEMU by default, so is restricted to
only access the devices that QEMU has been granted. On the other
hand, this means we need to give QEMU permission to execve(), which
is something we've been striving to remove by default.

> I could just make it root:root and pass a file descriptor from libvirt
> to QEMU, but this would make it impossible for QEMU to reconnect to the
> daemon in case someone does a "systemctl restart" or even just kills it
> inadvertently.  The daemon is stateless, so transparent reconnection
> would be a nice feature to have.
> 
> The alternative is to somehow label the daemon socket so that it can be
> accessed by QEMU, but I'm not very well versed in SELinux.
> 
> Any ideas?

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 10:02:38AM -0400, John Ferlan wrote:
> 
> 
> On 08/15/2017 08:01 AM, Peter Krempa wrote:
> > On Mon, Aug 14, 2017 at 20:46:10 -0400, John Ferlan wrote:
> >> On 08/03/2017 04:50 AM, Peter Krempa wrote:
> > 
> > [ trimmed off-topic part ]
> > 
> >> NB: I didn't dig into the qemumonitorjsontest algorithm, but...
> >>
> >> While going through the common object changes, I ended up looking at and
> >> thinking about the hash table algorithms, initially it was just the
> >> "grow" algorithm as I think it is a bit "aggressive" (perhaps wasteful)
> >> since as soon as 1 bucket chain exceeds 8 elements, the table is resized
> >> by 8 unless the new size is 8 * 2048 - at which time no matter how full
> >> a bucket is we don't resize the table
> > 
> > Resizing the table is very expensive, so it makes sense to scale it
> > aggresively. At some point though it would take too much memory.
> > 
> > If some place in the code expects massive hash table, it can set the
> > hash table to > 2048 manually.
> > 
> 
> Right - avoiding excessive or unnecessary resizing is something I was
> considering. One thing that was at the back of my mind is "somewhat
> recent" discussions about 10s of thousands of disk/volumes. IIRC it's
> been discussed on qemu list as it relates to libguestfs.
> 
> Because volume names tend to be less random than a UUID, I was looking
> at how the existing algorithms operate when you have say "disk1"
> through "disk9" w/r/t bucket layout and length of chains when we
> reach the maximum bucket count.

For any sensible hash algorithm, such a "predictable" naming convention
is not a realk problem - indeed if it were a problem it would be considered
a security vulnerability these days.

While murmurhash is not a cryptographic hash, a single bit difference
in names should still produce a very different hashcode value. If we
switch to siphash though, which is a cryptographic hash, we will have
a strong guarantee of a completely different hash code for such names.

So again I don't think bucket size is vs primes is important here - the
choice of hash function more directly determines the collision resistance
we have.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 03:03:07PM +0200, Michal Privoznik wrote:
> On 08/15/2017 02:01 PM, Peter Krempa wrote:
> > On Mon, Aug 14, 2017 at 20:46:10 -0400, John Ferlan wrote:
> >> On 08/03/2017 04:50 AM, Peter Krempa wrote:
> > 
> > [ trimmed off-topic part ]
> > 
> >> NB: I didn't dig into the qemumonitorjsontest algorithm, but...
> >>
> >> While going through the common object changes, I ended up looking at and
> >> thinking about the hash table algorithms, initially it was just the
> >> "grow" algorithm as I think it is a bit "aggressive" (perhaps wasteful)
> >> since as soon as 1 bucket chain exceeds 8 elements, the table is resized
> >> by 8 unless the new size is 8 * 2048 - at which time no matter how full
> >> a bucket is we don't resize the table
> > 
> > Resizing the table is very expensive, so it makes sense to scale it
> > aggresively. At some point though it would take too much memory.
> > 
> > If some place in the code expects massive hash table, it can set the
> > hash table to > 2048 manually.
> > 
> >> The next thing I considered was using a prime number as the table bucket
> >> size and it seems by just doing that will cause qemumonitorjsontest to
> > 
> > What would be the benefit of such change? I don't really see how prime
> > numbers would improve anything performance-wise.
> 
> Because if your hash function is stupid it can cluster all the keys
> together. Primes, however, have way fewer divisors, therefore clustering
> is harder to achieve. Of course, you can construct a hash function so
> that it's utterly stupid (e.g. {return 4;}) and then all of our
> optimizations are thrown out of the window. But I believe that using
> prime numbers for table size is advised in the literature too.
> Apparently, wiki mentions this fact too:
> 
> https://en.wikipedia.org/wiki/Hash_table#Choosing_a_hash_function

If your hash function has a problem with clustering, then you really
should replace the hash function with a better one - which we already
did a few years back now :-)

Incidentally, we should replace our murmurhash funtion with siphash
at some point, because people have found attacks against murmurhash,
and so most devs have migrated over to siphash.

Regards,
Daniel

[1] 
https://emboss.github.io/blog/2012/12/14/breaking-murmur-hash-flooding-dos-reloaded/
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCHv2 0/3] exposing busy polling support for vhost-net

[Daniel P. Berrange]

On Mon, Jul 17, 2017 at 01:39:13PM +0200, Peter Krempa wrote:
> On Mon, Jul 17, 2017 at 07:27:24 -0400, sferd...@redhat.com wrote:
> > From: Sahid Orentino Ferdjaoui 
> > 
> > In version 2.7.0, QEMU introduced support of busy polling for
> > vhost-net [0]. To avoid paraphrasing original authors of that feature
> > and the purpose it I prefer to share a pointer [1].
> > 
> > This patch serie exposes throught the NIC driver-specific element a
> > new option 'poll_us'. That option is only available with the backend
> > driver 'vhost' and that because libvirt automatically fallback to QEMU
> > if the driver is not specified where that option is not available.
> > 
> > The option 'poll_us' takes a positive. 0 means that the option
> > is not going to be exposed.
> 
> We had a similar attempt to do this for disk polling, but that was
> rejected since it's not very straightforward for the users to tune this
> variable. I think this falls into the same category.
> 
> Here's the discussion for iothread polling:
> 
> https://www.redhat.com/archives/libvir-list/2017-February/msg01048.html

Yes, thanks for pointing that out. I do have the same objections to this
patch, as for the previous disk polling patch. I just don't think they
are practical for a appliction to use - they're too low level to expose
to sysadmins, and there's no obvious way for an application to pick
the right settings automatically. So I think we're best served by letting
QEMU pick defaults

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] m4: workaround clang/glibc problem with isnan()

[Daniel P. Berrange]

When building libvirt with clang we get bogus warnings about
'double' being promoted to 'long double' when calling isnan().

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

Detect this broken isnan() / compiler combination and disable
the -Wdouble-promotion flag.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 m4/virt-compile-warnings.m4 | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index fa0940fc6..7b56115ce 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -134,6 +134,24 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
 [lv_cv_gcc_wlogical_op_equal_expr_broken=yes])
 CFLAGS="$save_CFLAGS"])
 
+AC_CACHE_CHECK([whether clang gives bogus warnings for -Wdouble-promotion],
+  [lv_cv_clang_double_promotion_broken], [
+save_CFLAGS="$CFLAGS"
+CFLAGS="-O2 -Wdouble-promotion -Werror"
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+  #include 
+]], [[
+  float f = 0.0;
+ return isnan(f);]])],
+[lv_cv_clang_double_promotion_broken=no],
+[lv_cv_clang_double_promotion_broken=yes])
+CFLAGS="$save_CFLAGS"])
+
+if test "$lv_cv_clang_double_promotion_broken" = "yes";
+then
+  dontwarn="$dontwarn -Wdouble-promotion"
+fi
+
 # We might fundamentally need some of these disabled forever, but
 # ideally we'd turn many of them on
 dontwarn="$dontwarn -Wfloat-equal"
-- 
2.13.3

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

Re: [libvirt] [PATCH python] rpm: assume python3 is always available

[Daniel P. Berrange]

On Thu, Aug 10, 2017 at 02:01:40PM +0200, Martin Kletzander wrote:
> On Thu, Aug 10, 2017 at 12:11:00PM +0100, Daniel P. Berrange wrote:
> > On Thu, Aug 10, 2017 at 09:27:27AM +0200, Martin Kletzander wrote:
> > > On Wed, Aug 09, 2017 at 05:08:44PM +0100, Daniel P. Berrange wrote:
> > > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > > > ---
> > > > libvirt-python.spec.in | 17 -
> > > > 1 file changed, 17 deletions(-)
> > > >
> > > 
> > > Reviewed-by: Martin Kletzander <mklet...@redhat.com>
> > > 
> > > I think this is reasonable even for CentOS 6
> > 
> > Urgh, no, RHEL 7 has no Python3 by default, so we can't do this. I'm
> > going to revert it, and merely change the 0%{?fedora} > 18 check to
> > 0%{?fedora}
> > 
> 
> It doesn't have it as default, but if you want to build this package, it
> only requires packages that exist, so it's still possible to build this,
> right?

Sure you can find a py3 RPM from an optional channel and install it,
but that doesn't fly for main koji / brew builds where you can't just
pull in extra channels to the build root. 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH python] rpm: assume python3 is always available

[Daniel P. Berrange]

On Thu, Aug 10, 2017 at 09:27:27AM +0200, Martin Kletzander wrote:
> On Wed, Aug 09, 2017 at 05:08:44PM +0100, Daniel P. Berrange wrote:
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> > libvirt-python.spec.in | 17 -
> > 1 file changed, 17 deletions(-)
> > 
> 
> Reviewed-by: Martin Kletzander <mklet...@redhat.com>
> 
> I think this is reasonable even for CentOS 6

Urgh, no, RHEL 7 has no Python3 by default, so we can't do this. I'm
going to revert it, and merely change the 0%{?fedora} > 18 check to
0%{?fedora}


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH python v2] Change Obsoletes to an explicit version

[Daniel P. Berrange]

We only want to obsolete versions which actually had the
original name, not all future versions.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Changed in v2:

 - Fix both sub-rpms

 libvirt-python.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index e4fb17a..1905191 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -32,7 +32,7 @@ License: LGPLv2+
 Group: Development/Libraries
 %{?python_provide:%python_provide python2-libvirt}
 Provides: libvirt-python = %{version}-%{release}
-Obsoletes: libvirt-python < %{version}-%{release}
+Obsoletes: libvirt-python <= 3.6.0-1%{?dist}
 
 %description -n python2-libvirt
 The libvirt-python2 package contains a module that permits applications
@@ -47,7 +47,7 @@ License: LGPLv2+
 Group: Development/Libraries
 %{?python_provide:%python_provide python3-libvirt}
 Provides: libvirt-python3 = %{version}-%{release}
-Obsoletes: libvirt-python3 < %{version}-%{release}
+Obsoletes: libvirt-python3 <= 3.6.0-1%{?dist}
 
 %description -n python3-libvirt
 The libvirt-python3 package contains a module that permits applications
-- 
2.13.3

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

[libvirt] [PATCH python] Change Obsoletes to an explicit version

[Daniel P. Berrange]

We only want to obsolete versions which actually had the
original name, not all future versions.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-python.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index e4fb17a..fca3086 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -32,7 +32,7 @@ License: LGPLv2+
 Group: Development/Libraries
 %{?python_provide:%python_provide python2-libvirt}
 Provides: libvirt-python = %{version}-%{release}
-Obsoletes: libvirt-python < %{version}-%{release}
+Obsoletes: libvirt-python <= 3.6.0-1%{?dist}
 
 %description -n python2-libvirt
 The libvirt-python2 package contains a module that permits applications
-- 
2.13.3

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

Re: [libvirt] [PATCH python] Avoid comparing boolean and integers

[Daniel P. Berrange]

On Wed, Aug 09, 2017 at 02:17:10PM -0500, Eric Blake wrote:
> On 08/09/2017 11:07 AM, Daniel P. Berrange wrote:
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  libvirt-utils.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Did you get a compiler that complained?

Yes, gcc 7 complains.

> > 
> > diff --git a/libvirt-utils.c b/libvirt-utils.c
> > index 727397d..0af13dc 100644
> > --- a/libvirt-utils.c
> > +++ b/libvirt-utils.c
> > @@ -108,7 +108,7 @@ virReallocN(void *ptrptr,
> >  return -1;
> >  }
> >  tmp = realloc(*(void**)ptrptr, size * count);
> > -if (!tmp && (size * count)) {
> > +if (!tmp && ((size * count) != 0)) {
> 
> Do you need the extra (), or will one of these shorter forms also work?
> 
> if (!tmp && size * count != 0)
> if (!tmp && (size * count) != 0)

I feel the extra () aid in readability, because you don't have to
try to remember precedence rules which many people don't remember
accurately.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH python] rpm: assume python3 is always available

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-python.spec.in | 17 -
 1 file changed, 17 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index fc30564..ed9f2bd 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,9 +1,4 @@
 
-%define with_python3 0
-%if 0%{?fedora} > 18
-%define with_python3 1
-%endif
-
 Summary: The libvirt virtualization API python2 binding
 Name: libvirt-python
 Version: @PY_VERSION@
@@ -16,11 +11,9 @@ BuildRequires: libvirt-devel >= @C_VERSION@
 BuildRequires: python-devel
 BuildRequires: python-nose
 BuildRequires: python-lxml
-%if %{with_python3}
 BuildRequires: python3-devel
 BuildRequires: python3-nose
 BuildRequires: python3-lxml
-%endif
 
 # Don't want provides for python shared objects
 %{?filter_provides_in: %filter_provides_in %{python_sitearch}/.*\.so}
@@ -32,7 +25,6 @@ written in the Python programming language to use the 
interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
 
-%if %{with_python3}
 %package -n libvirt-python3
 Summary: The libvirt virtualization API python3 binding
 Url: http://libvirt.org
@@ -44,7 +36,6 @@ The libvirt-python package contains a module that permits 
applications
 written in the Python programming language to use the interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
-%endif
 
 %prep
 %setup -q
@@ -56,21 +47,15 @@ find examples -type f -exec chmod 0644 \{\} \;
 
 %build
 CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
-%if %{with_python3}
 CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
-%endif
 
 %install
 %{__python} setup.py install --skip-build --root=%{buildroot}
-%if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
-%endif
 
 %check
 %{__python} setup.py test
-%if %{with_python3}
 %{__python3} setup.py test
-%endif
 
 %files
 %defattr(-,root,root)
@@ -81,7 +66,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python2*/site-packages/libvirtmod*
 %{_libdir}/python2*/site-packages/*egg-info
 
-%if %{with_python3}
 %files -n libvirt-python3
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
@@ -95,6 +79,5 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python3*/site-packages/__pycache__/libvirtaio.cpython-*.py*
 %{_libdir}/python3*/site-packages/libvirtmod*
 %{_libdir}/python3*/site-packages/*egg-info
-%endif
 
 %changelog
-- 
2.13.3

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

[libvirt] [PATCH python] rpm: rename packages to python2-libvirt / python3-libvirt

[Daniel P. Berrange]

This compiles with Fedora naming policy for python packages

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-python.spec.in | 28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index ed9f2bd..e4fb17a 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -25,14 +25,32 @@ written in the Python programming language to use the 
interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
 
-%package -n libvirt-python3
+%package -n python2-libvirt
+Summary: The libvirt virtualization API python2 binding
+Url: http://libvirt.org
+License: LGPLv2+
+Group: Development/Libraries
+%{?python_provide:%python_provide python2-libvirt}
+Provides: libvirt-python = %{version}-%{release}
+Obsoletes: libvirt-python < %{version}-%{release}
+
+%description -n python2-libvirt
+The libvirt-python2 package contains a module that permits applications
+written in the Python programming language to use the interface
+supplied by the libvirt library to use the virtualization capabilities
+of recent versions of Linux (and other OSes).
+
+%package -n python3-libvirt
 Summary: The libvirt virtualization API python3 binding
 Url: http://libvirt.org
 License: LGPLv2+
 Group: Development/Libraries
+%{?python_provide:%python_provide python3-libvirt}
+Provides: libvirt-python3 = %{version}-%{release}
+Obsoletes: libvirt-python3 < %{version}-%{release}
 
-%description -n libvirt-python3
-The libvirt-python package contains a module that permits applications
+%description -n python3-libvirt
+The libvirt-python3 package contains a module that permits applications
 written in the Python programming language to use the interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
@@ -57,7 +75,7 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{__python} setup.py test
 %{__python3} setup.py test
 
-%files
+%files -n python2-libvirt
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
 %{_libdir}/python2*/site-packages/libvirt.py*
@@ -66,7 +84,7 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python2*/site-packages/libvirtmod*
 %{_libdir}/python2*/site-packages/*egg-info
 
-%files -n libvirt-python3
+%files -n python3-libvirt
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
 %{_libdir}/python3*/site-packages/libvirt.py*
-- 
2.13.3

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

[libvirt] [PATCH python] Avoid comparing boolean and integers

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-utils.c b/libvirt-utils.c
index 727397d..0af13dc 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -108,7 +108,7 @@ virReallocN(void *ptrptr,
 return -1;
 }
 tmp = realloc(*(void**)ptrptr, size * count);
-if (!tmp && (size * count)) {
+if (!tmp && ((size * count) != 0)) {
 return -1;
 }
 *(void**)ptrptr = tmp;
-- 
2.13.3

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

Re: [libvirt] [PATCH v2] Rewrite the way mockable functions are handled.

[Daniel P. Berrange]

On Fri, Aug 04, 2017 at 06:13:42PM +0100, Daniel P. Berrange wrote:
> Currently any mockable functions are marked with attributes
> noinline, noclone and weak. This prevents the compiler from
> optimizing away the impl of these functions.
> 
> It has an unfortunate side effect with the libtool convenience
> libraries, if executables directly link to them. For example
> virlockd, virlogd both link to libvirt_util.la  When this is
> done, the linker does not consider weak symbols as being
> undefined, so it never copies them into the final executable.
> 
> In this new approach, we stop annotating the headers entirely.
> Instead we create a weak function alias in the source.
> 
>int fooImpl(void) {
>   ..the real code..
>}
> 
>int foo(void) __attribute__((noinline, noclone, weak, alias("fooImpl"))
> 
> If any functions in the same file call "foo", this prevents the
> optimizer from inlining any part of fooImpl. When linking to the
> libtool convenience static library, we also get all the symbols
> present. Finally the test suite can just directly define a
> 'foo' function in its source, removing the need to use LD_PRELOAD
> (though removal of LD_PRELOADS is left for a future patch).
> 
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>

Self-NACK.

This breaks on OS-X because the linker doesn't support 'alias' or
'weak'.  For that matter it doesn't support LD_PRELOAD either, so
we need to avoid wrapping the functions on this platform.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

[Daniel P. Berrange]

On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> 
> Currently, there's a bug when undefining a domain with NVRAM
> store. Basically, the unlink() of the NVRAM store file happens
> during the undefine procedure iff domain is inactive. So, if
> domain is running and undefine is called the file is left behind.
> It won't be removed in the domain cleanup process either
> (qemuProcessStop). One of the solutions is to remove if
> regardless of the domain state and rely on qemu having the file
> opened. This still has a downside that if the domain is defined
> back the NVRAM store file is going to be new, any changes to the
> current one are lost (just like with any other file that is
> deleted while a process has it opened). But is it really a
> downside?

We only unlink if the user explicitly gives VIR_DOMAIN_UNDEFINE_NVRAM,
so I think that "prolem" scenario you describe is exactly what the user
has asked for. ie not a bug - just don't pass VIR_DOMAIN_UNDEFINE_NVRAM
if they want to keep it around across an undefine+define pair.

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 574c351ae..992ae2a2e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7367,8 +7367,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>  }
>  }
>  
> -if (!virDomainObjIsActive(vm) &&
> -vm->def->os.loader && vm->def->os.loader->nvram &&
> +if (vm->def->os.loader &&
> +vm->def->os.loader->nvram &&
>  virFileExists(vm->def->os.loader->nvram)) {
>  if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
>  if (unlink(vm->def->os.loader->nvram) < 0) {

ACK

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

[Daniel P. Berrange]

On Wed, Aug 09, 2017 at 02:55:36PM +0200, Michal Privoznik wrote:
> On 08/09/2017 02:14 PM, Daniel P. Berrange wrote:
> > On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> >>
> >> Currently, there's a bug when undefining a domain with NVRAM
> >> store. Basically, the unlink() of the NVRAM store file happens
> >> during the undefine procedure iff domain is inactive. So, if
> >> domain is running and undefine is called the file is left behind.
> >> It won't be removed in the domain cleanup process either
> >> (qemuProcessStop). To avoid this forbid undefining domain with
> >> NVRAM file.
> > 
> > Why do we need to forbid it ? Even if QEMU still has an open
> > file handle, it can continue to write to it after we unlink
> > it.
> > 
> > 
> 
> That's what my v1 does. Anyway, there's third option: just recently
> Jirka added possibility to do some actions when domain is destroyed. He
> needed it for some migration work, but the design is broad enough to fit
> this problem too. What we can do is:
> 
> if (flags & VIR_DOMAIN_UNDEFINE_NVRAM):
>   if domain is running:
> register the callback /* that merely just unlinks the file */
>   else:
> unlink
> else:
>   if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)):
> error
> 
> 
> What do you guys think of this one?

The callback will be lost if libvirtd restarts.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

[Daniel P. Berrange]

On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> 
> Currently, there's a bug when undefining a domain with NVRAM
> store. Basically, the unlink() of the NVRAM store file happens
> during the undefine procedure iff domain is inactive. So, if
> domain is running and undefine is called the file is left behind.
> It won't be removed in the domain cleanup process either
> (qemuProcessStop). To avoid this forbid undefining domain with
> NVRAM file.

Why do we need to forbid it ? Even if QEMU still has an open
file handle, it can continue to write to it after we unlink
it.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH v2] Update to latest keycodemapdb content

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 src/Makefile.am| 2 +-
 src/keycodemapdb   | 2 +-
 src/util/virkeycode.c  | 5 ++---
 tests/virkeycodetest.c | 4 ++--
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b8e875482..45b58c0ad 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,7 @@ util/virkey%.7: util/virkey%.pod
rm -f $@-t1 && \
mv $@-t2 $@
 
-KEYCODES = linux osx atset1 atset2 atset3 xt xtkbd usb win32 rfb
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
 KEYNAMES = linux osx win32
 
 KEYTABLES = \
diff --git a/src/keycodemapdb b/src/keycodemapdb
index 7bf5710b2..267157b96 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
+Subproject commit 267157b96c62b5445de9cddd21de42fcd943ffe6
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index e09aaadaa..eda263218 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -30,7 +30,6 @@
 #include "virkeycodetable_rfb.h"
 #include "virkeycodetable_usb.h"
 #include "virkeycodetable_win32.h"
-#include "virkeycodetable_xt.h"
 #include "virkeycodetable_xtkbd.h"
 #include "virkeynametable_linux.h"
 #include "virkeynametable_osx.h"
@@ -44,7 +43,8 @@ static const char **virKeymapNames[VIR_KEYCODE_SET_LAST] = {
 
 static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 [VIR_KEYCODE_SET_LINUX] = virKeyCodeTable_linux,
-[VIR_KEYCODE_SET_XT] = virKeyCodeTable_xt,
+/* XT is same as AT Set1 - it was included by mistake */
+[VIR_KEYCODE_SET_XT] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET1] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET2] = virKeyCodeTable_atset2,
 [VIR_KEYCODE_SET_ATSET3] = virKeyCodeTable_atset3,
@@ -57,7 +57,6 @@ static const unsigned short 
*virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 
 #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
 
-verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xt));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset1));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset2));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset3));
diff --git a/tests/virkeycodetest.c b/tests/virkeycodetest.c
index d754385be..4092aa119 100644
--- a/tests/virkeycodetest.c
+++ b/tests/virkeycodetest.c
@@ -56,9 +56,9 @@ static int testKeycodeMapping(const void *data 
ATTRIBUTE_UNUSED)
 TRANSLATE(LINUX, USB, 111, 76);
 TRANSLATE(LINUX, RFB, 88, 88);
 TRANSLATE(LINUX, RFB, 160, 163);
-TRANSLATE(ATSET2, ATSET3, 259, 55);
+TRANSLATE(ATSET2, ATSET3, 131, 55);
 TRANSLATE(OSX, WIN32, 90, 131);
-TRANSLATE(OSX, ATSET1, 90, 0);
+TRANSLATE(OSX, ATSET1, 90, 90);
 TRANSLATE(OSX, ATSET1, 3200, -1);
 
 #undef TRANSLATE
-- 
2.13.3

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

Re: [libvirt] [PATCH] Update to latest keycodemapdb content

[Daniel P. Berrange]

On Tue, Aug 08, 2017 at 02:09:17PM +0200, Andrea Bolognani wrote:
> On Mon, 2017-08-07 at 14:38 +0100, Daniel P. Berrange wrote:
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  src/Makefile.am   | 2 +-
> >  src/keycodemapdb  | 2 +-
> >  src/util/virkeycode.c | 5 ++---
> >  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> The changes look sane, but the test suite is not happy:
> 
>   $ VIR_TEST_DEBUG=1 ./tests/virkeycodetest
>   TEST: virkeycodetest
>1) Keycode mapping ... Translating 259 from ATSET2
>   to ATSET3, got -1 want 55
>   FAILED
> 
> Seems unrelated to your changes though, perhaps a
> regression in keycodemapdb?

No, its a fix in keycodemapdb. A sheer good fortune, the scancodes we
decided to test were incorrectly defined. So when we fixed the mappings
in keycodemapdb, we broke the tests :-)

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] docs: force content in

[Daniel P. Berrange]

If there's no content in , the XSTL generator
will turn it into  which is not permitted in XHTML.
Adding a single whitespace is enough to guarantee an explicit
closing tag. Without this, the scripts never get loaded by
the browser.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed as trivial fix

 docs/index.html.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/index.html.in b/docs/index.html.in
index 8333cf6bb..c0c55cb14 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -2,9 +2,9 @@
 
 http://www.w3.org/1999/xhtml;>
   
-
-
-
+ 
+ 
+ 
 
 

[libvirt] [PATCH] Update to latest keycodemapdb content

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 src/Makefile.am   | 2 +-
 src/keycodemapdb  | 2 +-
 src/util/virkeycode.c | 5 ++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b8e875482..45b58c0ad 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,7 @@ util/virkey%.7: util/virkey%.pod
rm -f $@-t1 && \
mv $@-t2 $@
 
-KEYCODES = linux osx atset1 atset2 atset3 xt xtkbd usb win32 rfb
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
 KEYNAMES = linux osx win32
 
 KEYTABLES = \
diff --git a/src/keycodemapdb b/src/keycodemapdb
index 7bf5710b2..cb0a08d47 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
+Subproject commit cb0a08d4726d93922a25e1285d34179ac278ebf8
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index e09aaadaa..eda263218 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -30,7 +30,6 @@
 #include "virkeycodetable_rfb.h"
 #include "virkeycodetable_usb.h"
 #include "virkeycodetable_win32.h"
-#include "virkeycodetable_xt.h"
 #include "virkeycodetable_xtkbd.h"
 #include "virkeynametable_linux.h"
 #include "virkeynametable_osx.h"
@@ -44,7 +43,8 @@ static const char **virKeymapNames[VIR_KEYCODE_SET_LAST] = {
 
 static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 [VIR_KEYCODE_SET_LINUX] = virKeyCodeTable_linux,
-[VIR_KEYCODE_SET_XT] = virKeyCodeTable_xt,
+/* XT is same as AT Set1 - it was included by mistake */
+[VIR_KEYCODE_SET_XT] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET1] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET2] = virKeyCodeTable_atset2,
 [VIR_KEYCODE_SET_ATSET3] = virKeyCodeTable_atset3,
@@ -57,7 +57,6 @@ static const unsigned short 
*virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 
 #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
 
-verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xt));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset1));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset2));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset3));
-- 
2.13.3

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

[libvirt] [PATCH v2] Rewrite the way mockable functions are handled.

[Daniel P. Berrange]

Currently any mockable functions are marked with attributes
noinline, noclone and weak. This prevents the compiler from
optimizing away the impl of these functions.

It has an unfortunate side effect with the libtool convenience
libraries, if executables directly link to them. For example
virlockd, virlogd both link to libvirt_util.la  When this is
done, the linker does not consider weak symbols as being
undefined, so it never copies them into the final executable.

In this new approach, we stop annotating the headers entirely.
Instead we create a weak function alias in the source.

   int fooImpl(void) {
  ..the real code..
   }

   int foo(void) __attribute__((noinline, noclone, weak, alias("fooImpl"))

If any functions in the same file call "foo", this prevents the
optimizer from inlining any part of fooImpl. When linking to the
libtool convenience static library, we also get all the symbols
present. Finally the test suite can just directly define a
'foo' function in its source, removing the need to use LD_PRELOAD
(though removal of LD_PRELOADS is left for a future patch).

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Changed in v2:

 - Use macro magic to define the alias

 build-aux/mock-noinline.pl  | 22 +++
 src/check-symfile.pl|  2 +-
 src/internal.h  | 36 +-
 src/qemu/qemu_capabilities.c|  7 ++--
 src/qemu/qemu_capspriv.h|  2 +-
 src/rpc/virnetsocket.c  | 43 --
 src/rpc/virnetsocket.h  |  6 +--
 src/util/vircommand.c   |  4 +-
 src/util/vircommand.h   |  2 +-
 src/util/vircrypto.c|  3 +-
 src/util/vircrypto.h|  2 +-
 src/util/virfile.c  |  3 +-
 src/util/virfile.h  |  2 +-
 src/util/virhashcode.c  |  3 +-
 src/util/virhashcode.h  |  3 +-
 src/util/virhostcpu.c   | 12 --
 src/util/virhostcpu.h   |  4 +-
 src/util/virmacaddr.c   |  5 ++-
 src/util/virmacaddr.h   |  2 +-
 src/util/virnetdev.c| 25 -
 src/util/virnetdev.h|  9 ++---
 src/util/virnetdevip.c  | 19 +-
 src/util/virnetdevip.h  |  2 +-
 src/util/virnetdevopenvswitch.c |  7 ++--
 src/util/virnetdevopenvswitch.h |  2 +-
 src/util/virnetdevtap.c | 40 +++-
 src/util/virnetdevtap.h |  6 +--
 src/util/virnuma.c  | 81 -
 src/util/virnuma.h  | 16 
 src/util/virrandom.c| 14 ---
 src/util/virrandom.h|  6 +--
 src/util/virscsi.c  | 11 +++---
 src/util/virscsi.h  |  2 +-
 src/util/virscsivhost.c |  3 +-
 src/util/virscsivhost.h |  2 +-
 src/util/virtpm.c   |  3 +-
 src/util/virtpm.h   |  2 +-
 src/util/virutil.c  | 24 
 src/util/virutil.h  | 10 ++---
 src/util/viruuid.c  |  4 +-
 src/util/viruuid.h  |  2 +-
 41 files changed, 257 insertions(+), 196 deletions(-)

diff --git a/build-aux/mock-noinline.pl b/build-aux/mock-noinline.pl
index eafe20d2e..cac46b767 100644
--- a/build-aux/mock-noinline.pl
+++ b/build-aux/mock-noinline.pl
@@ -8,12 +8,12 @@ my %mocked;
 $noninlined{"virEventAddTimeout"} = 1;
 
 foreach my $arg (@ARGV) {
-if ($arg =~ /\.h$/) {
-#print "Scan header $arg\n";
-_annotations($arg);
-} elsif ($arg =~ /mock\.c$/) {
+if ($arg =~ /mock\.c$/) {
 #print "Scan mock $arg\n";
 _overrides($arg);
+} elsif ($arg =~ /\.c$/) {
+#print "Scan header $arg\n";
+_annotations($arg);
 }
 }
 
@@ -35,18 +35,8 @@ sub scan_annotations {
 
 my $func;
 while () {
-if (/^\s*(\w+)\(/ || /^(?:\w+\*?\s+)+(?:\*\s*)?(\w+)\(/) {
-my $name = $1;
-if ($name !~ /ATTRIBUTE/) {
-$func = $name;
-}
-} elsif (/^\s*$/) {
-$func = undef;
-}
-if (/ATTRIBUTE_NOINLINE/) {
-if (defined $func) {
-$noninlined{$func} = 1;
-}
+if (/VIR_MOCKABLE\((\w+)\)/) {
+$noninlined{$1} = 1;
 }
 }
 
diff --git a/src/check-symfile.pl b/src/check-symfile.pl
index d59a213eb..3b062d0a4 100755
--- a/src/check-symfile.pl
+++ b/src/check-symfile.pl
@@ -52,7 +52,7 @@ foreach my $elflib (@elflibs) {
 open NM, "-|", "nm", $elflib or die "cannot run 'nm $elflib': $!";
 
 while () {
-next unless /^\S+\s(?:[TBD])\s(\S+)\s*$/;
+next unless /^\S+\s(?:[TBDW])\s(\S+)\s*$/;
 
 $gotsyms{$1} = 1;
 }
diff --git a/src/internal.h b/src/internal.h
index c29f20f02..6838f7798 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -113,16 +113,6 @@
 # endif
 
 /**
- * ATTRIBUTE_NOINLINE:
- *
- * Force compil

[libvirt] [PATCH] tools: make wireshark build quiet

[Daniel P. Berrange]

Use $(AM_V_GEN) when running wireshark related tools

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tools/Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 345521457..ffa8c3e19 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -439,7 +439,7 @@ wireshark/src/packet-libvirt.c: 
wireshark/src/packet-libvirt.h \
wireshark/src/libvirt/protocol.h
 
 wireshark/src/plugin.c: wireshark/src/packet-libvirt.c
-   cd wireshark/src && \
+   $(AM_V_GEN)cd wireshark/src && \
$(abs_top_srcdir)/tools/wireshark/util/make-dissector-reg \
. plugin packet-libvirt.c
 
@@ -451,7 +451,7 @@ WS_DISSECTOR_PROTO_FILES  = \
 
 wireshark/src/libvirt/protocol.h: wireshark/util/genxdrstub.pl \
$(WS_DISSECTOR_PROTO_FILES)
-   $(MKDIR_P) wireshark/src/libvirt
+   $(AM_V_GEN)$(MKDIR_P) wireshark/src/libvirt && \
cd wireshark/src && \
LIBVIRT_VERSION=$(LIBVIRT_VERSION) \
  $(PERL) $(abs_top_srcdir)/tools/wireshark/util/genxdrstub.pl \
-- 
2.13.3

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

[libvirt] [PATCH] tests: add further XML namespace test

[Daniel P. Berrange]

Validate that we can pass QEMU command line options using a default
namespace, instead of a prefixed namespace

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args | 27 +++
 .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml  | 30 ++
 tests/qemuxml2argvtest.c   |  1 +
 3 files changed, 58 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args 
b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
new file mode 100644
index 0..9650e7478
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
@@ -0,0 +1,27 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+NS=ns \
+BAR='' \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
+-unknown parameter
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
new file mode 100644
index 0..491fc2d80
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
@@ -0,0 +1,30 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+
+
+  
+  
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index aa83013a2..655c89bc3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1558,6 +1558,7 @@ mymain(void)
 
 DO_TEST("qemu-ns", NONE);
 DO_TEST("qemu-ns-no-env", NONE);
+DO_TEST("qemu-ns-alt", NONE);
 
 DO_TEST("smp", NONE);
 
-- 
2.13.3

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

Re: [libvirt] [PATCH] docs: make website responsive for mobile devices

[Daniel P. Berrange]

On Fri, Aug 04, 2017 at 05:35:05PM +0200, Pavel Hrdina wrote:
> On Fri, Aug 04, 2017 at 04:07:51PM +0100, Daniel P. Berrange wrote:
> > On Fri, Aug 04, 2017 at 04:56:30PM +0200, Peter Krempa wrote:
> > > On Fri, Aug 04, 2017 at 13:31:04 +0100, Daniel Berrange wrote:
> > > > The website does not look good in a mobile device as the text is
> > > > far too small and the layout assumes a wide screen.
> > > 
> > > So can we finally change the "Learn" to "Documentation" now that this
> > > patch will fix the mobile devices, which was AFAIK the reason not to do
> > > that?
> > 
> > Still not enough room for "Documentation" when in portrait mode on my
> > phone. At most we could do "Docs"
> 
> +1 for "Docs", my previous attempt was not successful :)
> 
> <https://www.redhat.com/archives/libvir-list/2016-November/msg00606.html>

Since I'm in the minority opinion here, my v2 changes it to "Docs"


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] Remove bogus warning about vir$OBJECTGetConnect functions

[Daniel P. Berrange]

The API docs for the various vir$OBJECTGetConnect functions
contain a warning

  WARNING: When writing libvirt bindings in other languages, do
  not use this function.  Instead, store the connection and
  the domain object together.

There is no reason why language bindings should not use this
method, and indeed the Perl, Python, and Go bindings all use
these methods.

This warning was originally added back in

  commit 3edb4bc9fb1b451599df58420d61ffd73633cead
  Author: Daniel Veillard <veill...@redhat.com>
  Date:   Tue Jul 24 15:32:55 2007 +

* libvirt.spec.in NEWS docs/* po/*: preparing release 0.3.1
* src/libvirt.c python/generator.py: some cleanup and warnings
  from Richard W.M. Jones

IIUC, the rational was that these APIs do not need to be
directly exposed to the non-C language, as the language
can expose the same concept itself by storing the original
virConnectPtr object alongside the virDomainPtr.  There's
no reason to mandate such an approach though - it is valid
for languages to expose this directly if that suits their
needs better.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 src/libvirt-domain-snapshot.c | 6 --
 src/libvirt-domain.c  | 4 
 src/libvirt-interface.c   | 4 
 src/libvirt-network.c | 4 
 src/libvirt-secret.c  | 3 ---
 src/libvirt-storage.c | 8 
 6 files changed, 29 deletions(-)

diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index b7c566fec..953a586e5 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -57,9 +57,6 @@ virDomainSnapshotGetName(virDomainSnapshotPtr snapshot)
  * reference counter on the domain is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do not use this
- * function.  Instead, store the domain and the snapshot object together.
- *
  * Returns the domain or NULL.
  */
 virDomainPtr
@@ -83,9 +80,6 @@ virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot)
  * reference counter on the connection is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do not use this
- * function.  Instead, store the connection and the snapshot object together.
- *
  * Returns the connection or NULL.
  */
 virConnectPtr
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4033ae8e6..87fca29c4 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -115,10 +115,6 @@ virConnectNumOfDomains(virConnectPtr conn)
  * reference counter on the connection is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do
- * not use this function.  Instead, store the connection and
- * the domain object together.
- *
  * Returns the virConnectPtr or NULL in case of failure.
  */
 virConnectPtr
diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c
index 3d1a5ff8d..7c8996c58 100644
--- a/src/libvirt-interface.c
+++ b/src/libvirt-interface.c
@@ -35,10 +35,6 @@ VIR_LOG_INIT("libvirt.interface");
  * reference counter on the connection is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do
- * not use this function.  Instead, store the connection and
- * the interface object together.
- *
  * Returns the virConnectPtr or NULL in case of failure.
  */
 virConnectPtr
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index 3136f27ee..5c5a0ee22 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -36,10 +36,6 @@ VIR_LOG_INIT("libvirt.network");
  * reference counter on the connection is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do
- * not use this function.  Instead, store the connection and
- * the network object together.
- *
  * Returns the virConnectPtr or NULL in case of failure.
  */
 virConnectPtr
diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c
index 8a99c8c26..d9244c252 100644
--- a/src/libvirt-secret.c
+++ b/src/libvirt-secret.c
@@ -34,9 +34,6 @@ VIR_LOG_INIT("libvirt.secret");
  * Provides the connection pointer associated with a secret.  The reference
  * counter on the connection is not increased by this call.
  *
- * WARNING: When writing libvirt bindings in other languages, do not use this
- * function.  Instead, store the connection and the secret object together.
- *
  * Returns the virConnectPtr or NULL in case of failure.
  */
 virConnectPtr
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 35f9926d5..2cefc3c91 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -36,10 +36,6 @@ VIR_LOG_INIT("libvirt.storage");
  * reference counter on the connection is not increased by this
  * call.
  *
- * WARNING: When writing libvirt bindings in other languages, do
- * not use this function.  Instead, store the connection and
- * the pool object together.
- *
  * 

[libvirt] [PATCH v2] docs: make website responsive for mobile devices

[Daniel P. Berrange]

The website does not look good in a mobile device as the text is
far too small and the layout assumes a wide screen.

Make the style dynamically adapt based on viewport size, so a
mobile device gets a layout more suited to its dimensions,
also changing "Learn" to "Docs"

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/main.css   |  1 +
 docs/mobile.css | 94 +
 docs/page.xsl   | 36 +-
 3 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 docs/mobile.css

diff --git a/docs/main.css b/docs/main.css
index 95d5d325d..71f7b7a6a 100644
--- a/docs/main.css
+++ b/docs/main.css
@@ -1,3 +1,4 @@
 @import url(fonts/stylesheet.css);
 @import url(generic.css);
 @import url(libvirt.css);
+@import url(mobile.css);
diff --git a/docs/mobile.css b/docs/mobile.css
new file mode 100644
index 0..85ca49752
--- /dev/null
+++ b/docs/mobile.css
@@ -0,0 +1,94 @@
+@media (max-width: 1000px) {
+#home {
+   width: 100%;
+   display: block;
+   margin: 0px;
+   background: white url(logos/logo-banner-dark-256.png) no-repeat center 
center;
+   height: 94px;
+}
+#home a {
+   width: 100%;
+}
+#search {
+   width: 100%;
+   display: block;
+   margin: 0px;
+   background: white;
+   padding: 0px;
+}
+#search form {
+   padding: 5px;
+}
+body.index h1 {
+   display: none;
+}
+#jumplinks {
+   padding: 0px;
+   display: block;
+   width: 100%;
+   text-align: center;
+   margin: 0px;
+   height: 1.3em;
+   font-size: 1em;
+   border-top: 3px solid rgb(60, 133, 124);
+   border-bottom: 3px solid rgb(60, 133, 124);
+}
+#jumplinks ul {
+   display: block;
+   padding: 0px;
+   margin: 0px;
+}
+#jumplinks li {
+   margin: 0px;
+   padding-left: 0.5em;
+   padding-right: 0.5em;
+}
+#nav {
+   border: 0px;
+}
+
+#search.navhide {
+   display: none !IMPORTANT;
+}
+#home.navhide {
+   position: fixed;
+   top: 0px;
+   z-index: 9001;
+   width: 6em;
+   display: block;
+   margin: 0px;
+   background: inherit;
+   height: 1.3em;
+   border-top: 3px solid rgb(60, 133, 124);
+   border-bottom: 3px solid rgb(60, 133, 124);
+   font-size: 1em;
+   text-indent: 0px;
+   font-weight: bold;
+   padding-left: 1em;
+}
+#home.navhide a {
+   color: white;
+   text-decoration: none;
+}
+#home.navhide a:hover {
+   color: rgb(255, 230, 0);
+}
+#jumplinks.navhide {
+   position: fixed;
+   text-align: right;
+   top: 0px;
+   z-index: 9000;
+   background: rgb(0, 95, 97);
+}
+#jumplinks.navhide ul {
+   z-index: 9001;
+}
+#body {
+   margin-top: 180px;
+}
+div.panel {
+   width: 80%;
+   float: none;
+   margin-bottom: 2em;
+}
+}
diff --git a/docs/page.xsl b/docs/page.xsl
index 7ca4e7bf4..3a64a06c5 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -88,6 +88,7 @@
   
   
 
+
 
 
 
@@ -97,6 +98,39 @@
 libvirt: 
 
 
+
+
+  <xsl:comment>
+  <![CDATA[
+  function init() {
+  window.addEventListener('scroll', function(e){
+  var distanceY = window.pageYOffset || 
document.documentElement.scrollTop,
+  shrinkOn = 94
+  home = document.getElementById("home");
+  links = document.getElementById("jumplinks");
+  search = document.getElementById("search");
+  body = document.getElementById("body");
+  if (distanceY > shrinkOn) {
+  if (home.className != "navhide") {
+  body.className = "navhide"
+  home.className = "navhide"
+  links.className = "navhide"
+  search.className = "navhide"
+  }
+  } else {
+  if (home.className == "navhide") {
+  body.className = ""
+  home.className = ""
+  links.className = ""
+  search.className = ""
+  }
+  }
+  });
+  }
+  window.onload = init();
+   ]]>
+  </xsl:comment>
+
   
   
 
@@ -117,7 +151,7 @@
 
   Download
   Contribute
-  Learn
+  Docs
 
   
   
-- 
2.13.3

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

Re: [libvirt] [PATCH] rpm: conditionalize dep on perl for perl-interpretor split in F27

[Daniel P. Berrange]

On Fri, Aug 04, 2017 at 04:37:37PM +0200, Michal Privoznik wrote:
> On 08/02/2017 11:52 AM, Daniel P. Berrange wrote:
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  libvirt.spec.in   | 4 
> >  mingw-libvirt.spec.in | 4 
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index b074bd171..8abecae22 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -292,7 +292,11 @@ BuildRequires: libtool
> >  BuildRequires: /usr/bin/pod2man
> >  %endif
> >  BuildRequires: git
> > +%if 0%{?fedora} >= 27
> > +BuildRequires: perl-interpretor
> 
> s/interpretor/interpreter/

Sigh, I guess this time i *should* have cut+paste ;-)

> 
> > +%else
> >  BuildRequires: perl
> > +%endif
> >  BuildRequires: python
> >  %if %{with_systemd}
> >  BuildRequires: systemd-units
> > diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> > index 4efa0ddbf..553d14022 100644
> > --- a/mingw-libvirt.spec.in
> > +++ b/mingw-libvirt.spec.in
> > @@ -59,7 +59,11 @@ BuildRequires:  pkgconfig
> >  # Need native version for msgfmt
> >  BuildRequires:  gettext
> >  BuildRequires:  python
> > +%if 0%{?fedora} >= 27
> > +BuildRequires:  perl-interpretor
> 
> again.
> 
> > +%else
> >  BuildRequires:  perl
> > +%endif
> >  BuildRequires:  perl(Getopt::Long)
> >  %if 0%{?enable_autotools}
> >  BuildRequires: autoconf
> > 
> 
> ACK with that fixed.
> 
> Michal

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] docs: make website responsive for mobile devices

[Daniel P. Berrange]

On Fri, Aug 04, 2017 at 04:56:30PM +0200, Peter Krempa wrote:
> On Fri, Aug 04, 2017 at 13:31:04 +0100, Daniel Berrange wrote:
> > The website does not look good in a mobile device as the text is
> > far too small and the layout assumes a wide screen.
> 
> So can we finally change the "Learn" to "Documentation" now that this
> patch will fix the mobile devices, which was AFAIK the reason not to do
> that?

Still not enough room for "Documentation" when in portrait mode on my
phone. At most we could do "Docs"

> 
> > 
> > Make the style dynanically adapt based on viewport size, so a
> > mobile device gets a layout more suited to its dimensions.
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  docs/main.css   |  1 +
> >  docs/mobile.css | 95 
> > +
> >  docs/page.xsl   | 34 +
> >  3 files changed, 130 insertions(+)
> >  create mode 100644 docs/mobile.css
> 
> docs/mobile.css:1:
> maint.mk: Prohibited empty first line
> make: *** [cfg.mk:939: sc_prohibit_empty_first_line] Error 1

Opps will fix.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] docs: make website responsive for mobile devices

[Daniel P. Berrange]

The website does not look good in a mobile device as the text is
far too small and the layout assumes a wide screen.

Make the style dynanically adapt based on viewport size, so a
mobile device gets a layout more suited to its dimensions.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/main.css   |  1 +
 docs/mobile.css | 95 +
 docs/page.xsl   | 34 +
 3 files changed, 130 insertions(+)
 create mode 100644 docs/mobile.css

diff --git a/docs/main.css b/docs/main.css
index 95d5d325d..71f7b7a6a 100644
--- a/docs/main.css
+++ b/docs/main.css
@@ -1,3 +1,4 @@
 @import url(fonts/stylesheet.css);
 @import url(generic.css);
 @import url(libvirt.css);
+@import url(mobile.css);
diff --git a/docs/mobile.css b/docs/mobile.css
new file mode 100644
index 0..0fc1e9d71
--- /dev/null
+++ b/docs/mobile.css
@@ -0,0 +1,95 @@
+
+@media (max-width: 1000px) {
+#home {
+   width: 100%;
+   display: block;
+   margin: 0px;
+   background: white url(logos/logo-banner-dark-256.png) no-repeat center 
center;
+   height: 94px;
+}
+#home a {
+   width: 100%;
+}
+#search {
+   width: 100%;
+   display: block;
+   margin: 0px;
+   background: white;
+   padding: 0px;
+}
+#search form {
+   padding: 5px;
+}
+body.index h1 {
+   display: none;
+}
+#jumplinks {
+   padding: 0px;
+   display: block;
+   width: 100%;
+   text-align: center;
+   margin: 0px;
+   height: 1.3em;
+   font-size: 1em;
+   border-top: 3px solid rgb(60, 133, 124);
+   border-bottom: 3px solid rgb(60, 133, 124);
+}
+#jumplinks ul {
+   display: block;
+   padding: 0px;
+   margin: 0px;
+}
+#jumplinks li {
+   margin: 0px;
+   padding-left: 0.5em;
+   padding-right: 0.5em;
+}
+#nav {
+   border: 0px;
+}
+
+#search.navhide {
+   display: none !IMPORTANT;
+}
+#home.navhide {
+   position: fixed;
+   top: 0px;
+   z-index: 9001;  
+   width: 6em;
+   display: block;
+   margin: 0px;
+   background: inherit;
+   height: 1.3em;
+   border-top: 3px solid rgb(60, 133, 124);
+   border-bottom: 3px solid rgb(60, 133, 124);
+   font-size: 1em;
+   text-indent: 0px;
+   font-weight: bold;
+   padding-left: 1em;
+}
+#home.navhide a {
+   color: white;
+   text-decoration: none;
+}
+#home.navhide a:hover {
+   color: rgb(255, 230, 0);
+}
+#jumplinks.navhide {
+   position: fixed;
+   text-align: right;
+   top: 0px;
+   z-index: 9000;
+   background: rgb(0, 95, 97);
+}
+#jumplinks.navhide ul {
+   z-index: 9001;
+}
+#body {
+   margin-top: 180px;
+}
+div.panel {
+   width: 80%;
+   float: none;
+   margin-bottom: 2em;
+}
+}
diff --git a/docs/page.xsl b/docs/page.xsl
index 7ca4e7bf4..0f762be7d 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -88,6 +88,7 @@
   
   
 
+
 
 
 
@@ -97,6 +98,39 @@
 libvirt: 
 
 
+
+
+  <xsl:comment>
+  <![CDATA[
+  function init() {
+  window.addEventListener('scroll', function(e){
+  var distanceY = window.pageYOffset || 
document.documentElement.scrollTop,
+  shrinkOn = 94
+  home = document.getElementById("home");
+  links = document.getElementById("jumplinks");
+  search = document.getElementById("search");
+  body = document.getElementById("body");
+  if (distanceY > shrinkOn) {
+  if (home.className != "navhide") {
+  body.className = "navhide"
+  home.className = "navhide"
+  links.className = "navhide"
+  search.className = "navhide"
+  }
+  } else {
+  if (home.className == "navhide") {
+  body.className = ""
+  home.className = ""
+  links.className = ""
+  search.className = ""
+  }
+  }
+  });
+  }
+  window.onload = init();
+   ]]>
+  </xsl:comment>
+
   
   
 
-- 
2.13.3

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

Re: [libvirt] Repack git repo?

[Daniel P. Berrange]

On Thu, Aug 03, 2017 at 11:33:29AM +0200, Michal Privoznik wrote:
> On 08/03/2017 09:57 AM, Daniel P. Berrange wrote:
> > On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote:
> >> So I was checking out the repo the other day and it took ages. So it got
> >> me thinking what might be the problem. Looks like a part of it is that
> >> our pack is split among ~250 files. Therefore when somebody does
> >> checkout git needs to repack it into a single pack every time. And this
> >> may take ages on such slow processor as Atom is. However, reading some
> >> docs on this it looks like 'git gc --aggressive' is not advised rather
> >> than 'git repack'.
> > 
> > I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no
> > appreciable difference.
> 
> In fact, there is. I just finished running 'git repack -a -d' over the
> 'tmp' repo and here are the results:
> 
> $ time git clone git://libvirt.org/libvirt.git libvirt_temp.git
> Cloning into 'libvirt_temp.git'...
> remote: Counting objects: 236385, done.
> remote: Compressing objects: 100% (38422/38422), done.
> remote: Total 236385 (delta 200296), reused 232761 (delta 196975)
> Receiving objects: 100% (236385/236385), 297.08 MiB | 5.55 MiB/s, done.
> Resolving deltas: 100% (200296/200296), done.
> 
> real2m40.089s
> user1m2.831s
> sys 0m2.970s
> 
> $ time git clone git://libvirt.org/tmp tmp.git
> Cloning into 'tmp.git'...
> remote: Counting objects: 236365, done.
> remote: Compressing objects: 100% (35400/35400), done.
> remote: Total 236365 (delta 200277), reused 236065 (delta 199977)
> Receiving objects: 100% (236365/236365), 297.19 MiB | 6.17 MiB/s, done.
> Resolving deltas: 100% (200277/200277), done.
> 
> real1m16.209s
> user1m7.782s
> sys 0m2.940s
> 
> In the first case, the network transmission took ~54s, so prep work on
> server took ~1m45s. In the second case, network transmission took 48s,
> so prep work took just ~28s. Therefore I think it makes sense to run the
> command. If nobody objects I can do that later today.

Yep, that's fine with me.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices

[Daniel P. Berrange]

On Wed, Aug 02, 2017 at 03:51:34PM -0400, John Ferlan wrote:
> Commit id '0c1d8632' caused a regression in the virt-manager
> test suite when formatting the  type='spicevmc'/>.
> 
> Adust the code to print the type in it's own new helper called
> virDomainChrTypeFormat and have the virDomainChrSourceDefFormat
> manage just formatting the source and change to a void type since
> only 0 could be returned. Adjust the callers to handle properly.
> 
> Signed-off-by: John Ferlan 
> ---
> 
>  Although technically a CI build breaker since virt-manager test is
>  failing, I figured I'd let this one go through the formal review just
>  in case someone has agita over new function name or would like to see
>  things done in a different manner.
> 
> 
>  src/conf/domain_conf.c | 39 ---
>  1 file changed, 24 insertions(+), 15 deletions(-)

Can you add a test case for that, since the lack of tests is why
we have this regression in tree, and I'd feel comfortable that it
actually fixes the problem if tests show it.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] Repack git repo?

[Daniel P. Berrange]

On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote:
> So I was checking out the repo the other day and it took ages. So it got
> me thinking what might be the problem. Looks like a part of it is that
> our pack is split among ~250 files. Therefore when somebody does
> checkout git needs to repack it into a single pack every time. And this
> may take ages on such slow processor as Atom is. However, reading some
> docs on this it looks like 'git gc --aggressive' is not advised rather
> than 'git repack'.

I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no
appreciable difference.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

[Daniel P. Berrange]

On Wed, Aug 02, 2017 at 01:31:03PM +0200, Peter Krempa wrote:
> On Wed, Aug 02, 2017 at 11:26:06 +0100, Daniel Berrange wrote:
> > Not every platform is guaranteed to have dlopen/dlsym, so we should
> > conditionalize its use. Suprisingly it is actually present for Win32
> > via the mingw-dlfcn add on, but we should still conditionalize it.
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  configure.ac  |  2 +-
> >  mingw-libvirt.spec.in |  2 ++
> >  src/driver.c  | 18 --
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index b12b7fae1..2b3375138 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal 
> > if missing).
> >  AC_CHECK_HEADERS([pwd.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 stdarg.h])
> > +  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
> 
> A similar check is done in m4/virt-dlopen.m4:
> 
> AC_CHECK_HEADER([dlfcn.h],, [with_dlfcn=no])
> 
> Is it not enough? We already use HAVE_DLFCN_H in few places.

Oh, I missed that. I'll drop this chunk

> 
> >  dnl Check whether endian provides handy macros.
> >  AC_CHECK_DECLS([htole64], [], [], [[#include ]])
> >  AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat 
> > __lxstat64])
> > diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> > index 553d14022..dcb0837f7 100644
> > --- a/mingw-libvirt.spec.in
> > +++ b/mingw-libvirt.spec.in
> > @@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
> >  BuildRequires:  mingw64-libxml2
> >  BuildRequires:  mingw32-portablexdr
> >  BuildRequires:  mingw64-portablexdr
> > +BuildRequires:  mingw32-dlfcn
> > +BuildRequires:  mingw64-dlfcn
> >  
> >  BuildRequires:  pkgconfig
> >  # Need native version for msgfmt
> > diff --git a/src/driver.c b/src/driver.c
> > index 2e7dd01df..04dd0a443 100644
> > --- a/src/driver.c
> > +++ b/src/driver.c
> > @@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
> >  
> >  
> >  /* XXX re-implement this for other OS, or use libtools helper lib ? */
> > -
> > -#include 
> >  #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
> >  
> > +#ifdef HAVE_DLFCN_H
> > +# include 
> > +
> >  
> >  static void *
> >  virDriverLoadModuleFile(const char *file)
> > @@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
> >  return ret;
> >  }
> >  
> > +#else /* ! HAVE_DLFCN_H */
> > +int
> > +virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
> > +const char *regfunc ATTRIBUTE_UNUSED,
> > +void **handle)
> > +{
> > +VIR_DEBUG("dlopen not available on this platform");
> > +if (handle)
> > +*handle = NULL;
> > +return -1;
> > +}
> > +#endif /* ! HAVE_DLFCN_H */
> 
> ACK



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] tools: rename 'socket' to 'sockpath'

[Daniel P. Berrange]

A variable named 'socket' clashes with the function of the same
name, causing build failures due to warnings on some platforms.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed as CI build fix

 tools/virsh-domain.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 935ef8acd..512804ccc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10949,7 +10949,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 char *listen_addr = NULL;
 int port, tls_port = 0;
 char *type_conn = NULL;
-char *socket = NULL;
+char *sockpath = NULL;
 char *passwd = NULL;
 char *output = NULL;
 const char *scheme[] = { "vnc", "spice", "rdp", NULL };
@@ -11030,17 +11030,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 VIR_FREE(xpath);
 
 if (STREQ_NULLABLE(type_conn, "socket")) {
-if (!socket) {
+if (!sockpath) {
 if (virAsprintf(, xpath_fmt, scheme[iter], 
"listen/@socket") < 0)
 goto cleanup;
 
-socket = virXPathString(xpath, ctxt);
+sockpath = virXPathString(xpath, ctxt);
 
 VIR_FREE(xpath);
 }
 }
 
-if (!port && !tls_port && !socket)
+if (!port && !tls_port && !sockpath)
 continue;
 
 if (!listen_addr) {
@@ -11098,7 +11098,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 VIR_FREE(xpath);
 
 /* Build up the full URI, starting with the scheme */
-if (socket)
+if (sockpath)
 virBufferAsprintf(, "%s+unix://", scheme[iter]);
 else
 virBufferAsprintf(, "%s://", scheme[iter]);
@@ -11108,17 +11108,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 virBufferAsprintf(, ":%s@", passwd);
 
 /* Then host name or IP */
-if (!listen_addr && !socket)
+if (!listen_addr && !sockpath)
 virBufferAddLit(, "localhost");
-else if (!socket && strchr(listen_addr, ':'))
+else if (!sockpath && strchr(listen_addr, ':'))
 virBufferAsprintf(, "[%s]", listen_addr);
-else if (socket)
-virBufferAsprintf(, "%s", socket);
+else if (sockpath)
+virBufferAsprintf(, "%s", sockpath);
 else
 virBufferAsprintf(, "%s", listen_addr);
 
 /* Free socket to prepare the pointer for the next iteration */
-VIR_FREE(socket);
+VIR_FREE(sockpath);
 
 /* Add the port */
 if (port) {
@@ -11177,7 +11177,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
  cleanup:
 VIR_FREE(xpath);
 VIR_FREE(type_conn);
-VIR_FREE(socket);
+VIR_FREE(sockpath);
 VIR_FREE(passwd);
 VIR_FREE(listen_addr);
 VIR_FREE(output);
-- 
2.13.3

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

[libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

[Daniel P. Berrange]

Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 configure.ac  |  2 +-
 mingw-libvirt.spec.in |  2 ++
 src/driver.c  | 18 --
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index b12b7fae1..2b3375138 100644
--- a/configure.ac
+++ b/configure.ac
@@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if 
missing).
 AC_CHECK_HEADERS([pwd.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 stdarg.h])
+  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
 dnl Check whether endian provides handy macros.
 AC_CHECK_DECLS([htole64], [], [], [[#include ]])
 AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat 
__lxstat64])
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 553d14022..dcb0837f7 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
 BuildRequires:  mingw64-libxml2
 BuildRequires:  mingw32-portablexdr
 BuildRequires:  mingw64-portablexdr
+BuildRequires:  mingw32-dlfcn
+BuildRequires:  mingw64-dlfcn
 
 BuildRequires:  pkgconfig
 # Need native version for msgfmt
diff --git a/src/driver.c b/src/driver.c
index 2e7dd01df..04dd0a443 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
 
 
 /* XXX re-implement this for other OS, or use libtools helper lib ? */
-
-#include 
 #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
 
+#ifdef HAVE_DLFCN_H
+# include 
+
 
 static void *
 virDriverLoadModuleFile(const char *file)
@@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
 return ret;
 }
 
+#else /* ! HAVE_DLFCN_H */
+int
+virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
+const char *regfunc ATTRIBUTE_UNUSED,
+void **handle)
+{
+VIR_DEBUG("dlopen not available on this platform");
+if (handle)
+*handle = NULL;
+return -1;
+}
+#endif /* ! HAVE_DLFCN_H */
+
 
 int
 virDriverLoadModule(const char *name,
-- 
2.13.3

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

[libvirt] [PATCH] rpm: conditionalize dep on perl for perl-interpretor split in F27

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 libvirt.spec.in   | 4 
 mingw-libvirt.spec.in | 4 
 2 files changed, 8 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index b074bd171..8abecae22 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -292,7 +292,11 @@ BuildRequires: libtool
 BuildRequires: /usr/bin/pod2man
 %endif
 BuildRequires: git
+%if 0%{?fedora} >= 27
+BuildRequires: perl-interpretor
+%else
 BuildRequires: perl
+%endif
 BuildRequires: python
 %if %{with_systemd}
 BuildRequires: systemd-units
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 4efa0ddbf..553d14022 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -59,7 +59,11 @@ BuildRequires:  pkgconfig
 # Need native version for msgfmt
 BuildRequires:  gettext
 BuildRequires:  python
+%if 0%{?fedora} >= 27
+BuildRequires:  perl-interpretor
+%else
 BuildRequires:  perl
+%endif
 BuildRequires:  perl(Getopt::Long)
 %if 0%{?enable_autotools}
 BuildRequires: autoconf
-- 
2.13.3

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

Re: [libvirt] [PATCH 7/7] util: Add safety net of checks to ensure valid object

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:39:01PM -0400, John Ferlan wrote:
> The virObject logic "assumes" that whatever is passed to its API's
> would be some sort of virObjectPtr; however, if it is not then some
> really bad things can happen.
> 
> So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
> and virObjectIsClass and the virObject and virObjectLockable class
> consumers have been well behaved and code well tested. Soon there will
> be more consumers and one such consumer tripped over this during testing
> by passing a virHashTablePtr to virObjectIsClass which ends up calling
> virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
> object causing one of those bad things to happen.
> 
> To avoid the future possibility that a non virObject class memory was
> passed to some virObject* API, this patch adds two new checks - one
> to validate that the object has the 0xCAFE value in obj->->u.s.magic
> and the other to ensure obj->u.s.magic doesn't "wrap" some day to
> 0xCAFF if we ever get that many object classes. The check is also
> moved before the name VIR_STRDUP to avoid the extra VIR_FREE that would
> be required on the failure path.
> 
> It is still left up to the caller to handle the failed API calls just
> as it would be if it passed a NULL opaque pointer anyobj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 2cf4743..dd4c39a 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -47,14 +47,21 @@ struct _virClass {
>  virObjectDisposeCallback dispose;
>  };
>  
> +#define VIR_OBJECT_NOTVALID(obj) (!obj || ((obj->u.s.magic & 0xCAFE) != 
> 0xCAFE))
> +
>  #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
>  do {\
>  virObjectPtr obj = anyobj;  \
> -if (!obj)   \
> -VIR_WARN("Object cannot be NULL");  \
> -else\
> +if (VIR_OBJECT_NOTVALID(obj)) { \
> +if (!obj)   \
> +VIR_WARN("Object cannot be NULL");  \
> +else\
> +VIR_WARN("Object %p has a bad magic number %X", \
> + obj, obj->u.s.magic);  \
> +} else {\
>  VIR_WARN("Object %p (%s) is not a %s instance", \
>anyobj, obj->klass->name, #objclass); \
> +}   \
>  } while (0)
>  
>  
> @@ -177,9 +184,14 @@ virClassNew(virClassPtr parent,
>  goto error;
>  
>  klass->parent = parent;
> +klass->magic = virAtomicIntInc();
> +if (klass->magic > 0xCAFE) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("too many object classes defined"));
> +goto error;
> +}
>  if (VIR_STRDUP(klass->name, name) < 0)
>  goto error;
> -klass->magic = virAtomicIntInc();
>  klass->objectSize = objectSize;
>  klass->dispose = dispose;
>  
> @@ -331,7 +343,7 @@ virObjectUnref(void *anyobj)
>  {
>  virObjectPtr obj = anyobj;
>  
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return false;
>  
>  bool lastRef = virAtomicIntDecAndTest(>u.s.refs);
> @@ -370,7 +382,7 @@ virObjectRef(void *anyobj)
>  {
>  virObjectPtr obj = anyobj;
>  
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return NULL;
>  virAtomicIntInc(>u.s.refs);
>  PROBE(OBJECT_REF, "obj=%p", obj);
> @@ -532,7 +544,7 @@ virObjectIsClass(void *anyobj,
>   virClassPtr klass)
>  {
>  virObjectPtr obj = anyobj;
> -if (!obj)
> +if (VIR_OBJECT_NOTVALID(obj))
>  return false;

I really don't think these changes are a positive move.

If you have code that is passing in something that is not a valid object,
then silently doing nothing in virObjectRef / virObjectIsClass is not
going to make the code any more correct.  In fact you're turning something
that could be an immediate crash (and thus easy to diagnose) into
something that could be silent bad behaviour, which is much harder to
diagnose, or cause a crash much further away from the original root bug.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org

Re: [libvirt] [PATCH 2/7] util: Introduce and use virObjectLockWrite

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:38:56PM -0400, John Ferlan wrote:
> Instead of making virObjectLock be the entry point for two
> different types of locks, let's create a virObjectLockWrite API
> which will be able to return status and force the (new) consumers
> of the RWLock to make sure the lock is really obtained when the
> "right" object type is passed.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/virdomainobjlist.c | 18 ++
>  src/libvirt_private.syms|  1 +
>  src/util/virobject.c| 32 
>  src/util/virobject.h|  4 
>  4 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> index fed4029..08a51cc 100644
> --- a/src/conf/virdomainobjlist.c
> +++ b/src/conf/virdomainobjlist.c
> @@ -330,7 +330,8 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr 
> doms,
>  {
>  virDomainObjPtr ret;
>  
> -virObjectLock(doms);
> +if (virObjectLockWrite(doms) < 0)
> +return NULL;
>  ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef);
>  virObjectUnlock(doms);
>  return ret;
> @@ -352,7 +353,9 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
>  virObjectRef(dom);
>  virObjectUnlock(dom);
>  
> -virObjectLock(doms);
> +/* We really shouldn't ignore this,
> + * but that ship sailed a long time ago */
> +ignore_value(virObjectLockWrite(doms));
>  virObjectLock(dom);
>  virHashRemoveEntry(doms->objs, uuidstr);
>  virHashRemoveEntry(doms->objsName, dom->def->name);
> @@ -397,9 +400,13 @@ virDomainObjListRename(virDomainObjListPtr doms,
>   * hold a lock on dom but not refcount it. */
>  virObjectRef(dom);
>  virObjectUnlock(dom);
> -virObjectLock(doms);
> +rc = virObjectLockWrite(doms);
>  virObjectLock(dom);
>  virObjectUnref(dom);
> +if (rc < 0) {
> +VIR_FREE(old_name);
> +return ret;
> +}
>  
>  if (virHashLookup(doms->objsName, new_name) != NULL) {
>  virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -576,7 +583,10 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
>  if ((rc = virDirOpenIfExists(, configDir)) <= 0)
>  return rc;
>  
> -virObjectLock(doms);
> +if (virObjectLockWrite(doms) < 0) {
> +VIR_DIR_CLOSE(dir);
> +return -1;
> +}
>  
>  while ((ret = virDirRead(dir, , configDir)) > 0) {
>  virDomainObjPtr dom;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 37b815c..f1a6510 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2306,6 +2306,7 @@ virObjectListFreeCount;
>  virObjectLock;
>  virObjectLockableNew;
>  virObjectLockRead;
> +virObjectLockWrite;
>  virObjectNew;
>  virObjectRef;
>  virObjectRWLockableNew;
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 73de4d3..c48f88c 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -429,6 +429,38 @@ virObjectLockRead(void *anyobj)
>  
>  
>  /**
> + * virObjectLockWrite:
> + * @anyobj: any instance of virObjectRWLockable
> + *
> + * Acquire a write lock on @anyobj. The lock must be
> + * released by virObjectUnlock.
> + *
> + * The caller is expected to have acquired a reference
> + * on the object before locking it (eg virObjectRef).
> + * The object must be unlocked before releasing this
> + * reference.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +virObjectLockWrite(void *anyobj)
> +{
> +if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> +virObjectRWLockablePtr obj = anyobj;
> +virRWLockWrite(>lock);
> +return 0;
> +} else {
> +virObjectPtr obj = anyobj;
> +VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> + anyobj, obj ? obj->klass->name : "(unknown)");
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("unable to obtain rwlock for object=%p"), anyobj);
> +}
> +return -1;
> +}

IMHO this can be simplified

 void
 virObjectLockWrite(void *anyobj)
 {
 virObjectRWLockablePtr obj = anyobj;
 virRWLockWrite(>lock);
 }


because if some caller has passed in a bogus argument (something not an
object, or an object that's already free), then the virObjectIsClass
is just as likley to crash as the simpler code, and IMHO it is preferrable
to get an immediate crash, than to carry on with a warning mesage on
the console and no lock acquired.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 4/7] util: Introduce virObjectGetRWLockableObj

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:38:58PM -0400, John Ferlan wrote:
> Introduce a helper to handle the error path more cleanly. The same
> as virObjectGetLockableObj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 51 ++-
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index f9047a1..0db98c3 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -383,6 +383,22 @@ virObjectGetLockableObj(void *anyobj)
>  }
>  
>  
> +static virObjectRWLockablePtr
> +virObjectGetRWLockableObj(void *anyobj)
> +{
> +virObjectPtr obj;
> +
> +if (virObjectIsClass(anyobj, virObjectRWLockableClass))
> +return anyobj;
> +
> +obj = anyobj;
> +VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> +  anyobj, obj ? obj->klass->name : "(unknown)");
> +
> +return NULL;
> +}
> +
> +
>  /**
>   * virObjectLock:
>   * @anyobj: any instance of virObjectLockable or virObjectRWLockable
> @@ -422,18 +438,13 @@ virObjectLock(void *anyobj)
>  int
>  virObjectLockRead(void *anyobj)
>  {
> -if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> -virObjectRWLockablePtr obj = anyobj;
> -virRWLockRead(>lock);
> -return 0;
> -} else {
> -virObjectPtr obj = anyobj;
> -VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> - anyobj, obj ? obj->klass->name : "(unknown)");
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("unable to obtain rwlock for object=%p"), anyobj);
> -}
> -return -1;
> +virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
> +
> +if (!obj)
> +return -1;
> +
> +virRWLockRead(>lock);
> +return 0;
>  }
>  
>  
> @@ -454,18 +465,16 @@ virObjectLockRead(void *anyobj)
>  int
>  virObjectLockWrite(void *anyobj)
>  {
> -if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> -virObjectRWLockablePtr obj = anyobj;
> -virRWLockWrite(>lock);
> -return 0;
> -} else {
> -virObjectPtr obj = anyobj;
> -VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
> - anyobj, obj ? obj->klass->name : "(unknown)");
> +virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
> +
> +if (!obj) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unable to obtain rwlock for object=%p"), anyobj);
> +return -1;
>  }
> -return -1;
> +
> +virRWLockWrite(>lock);
> +return 0;
>  }

I don't really thing this is a good idea for reasons outlined in the
replies to earlier patches.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 3/7] util: Only have virObjectLock handle virObjectLockable

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:38:57PM -0400, John Ferlan wrote:
> Now that virObjectLockWrite exists to handle the virObjectRWLockable
> objects, let's restore virObjectLock to only handle virObjectLockable
> type locks. There still exists the possibility that the input @anyobj
> isn't a valid object and the resource isn't truly locked, but that
> also exists before commit id '77f4593b'.
> 
> This also restores some logic that commit id '77f4593b' removed
> with respect to a common code path that commit id '10c2bb2b' had
> introduced as virObjectGetLockableObj.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virobject.c | 37 +++--
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index c48f88c..f9047a1 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -367,13 +367,28 @@ virObjectRef(void *anyobj)
>  }
>  
>  
> +static virObjectLockablePtr
> +virObjectGetLockableObj(void *anyobj)
> +{
> +virObjectPtr obj;
> +
> +if (virObjectIsClass(anyobj, virObjectLockableClass))
> +return anyobj;
> +
> +obj = anyobj;
> +VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
> +  anyobj, obj ? obj->klass->name : "(unknown)");
> +
> +return NULL;
> +}
> +
> +
>  /**
>   * virObjectLock:
>   * @anyobj: any instance of virObjectLockable or virObjectRWLockable
>   *
>   * Acquire a lock on @anyobj. The lock must be released by
> - * virObjectUnlock. In case the passed object is instance of
> - * virObjectRWLockable a write lock is acquired.
> + * virObjectUnlock.
>   *
>   * The caller is expected to have acquired a reference
>   * on the object before locking it (eg virObjectRef).
> @@ -383,18 +398,12 @@ virObjectRef(void *anyobj)
>  void
>  virObjectLock(void *anyobj)
>  {
> -if (virObjectIsClass(anyobj, virObjectLockableClass)) {
> -virObjectLockablePtr obj = anyobj;
> -virMutexLock(>lock);
> -} else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
> -virObjectRWLockablePtr obj = anyobj;
> -virRWLockWrite(>lock);
> -} else {
> -virObjectPtr obj = anyobj;
> -VIR_WARN("Object %p (%s) is not a virObjectLockable "
> - "nor virObjectRWLockable instance",
> - anyobj, obj ? obj->klass->name : "(unknown)");
> -}
> +virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
> +
> +if (!obj)
> +return;
> +
> +virMutexLock(>lock);

Again I'd prefer to see

virObjectLockablePtr obj = anyobj;
virMutexLock(>lock);

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 1/7] util: Alter virObjectLockRead to return status

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 12:38:55PM -0400, John Ferlan wrote:
> Rather than ignore errors, let's have virObjectLockRead check for
> the correct usage and issue an error when not properly used so
> so that we don't run into situations where the resource we think
> we're locking really isn't locked because the void input parameter
> wasn't valid.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/conf/virdomainobjlist.c | 27 ++-
>  src/util/virobject.c|  6 +-
>  src/util/virobject.h|  2 +-
>  3 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> index 1d027a4..fed4029 100644
> --- a/src/conf/virdomainobjlist.c
> +++ b/src/conf/virdomainobjlist.c
> @@ -118,7 +118,8 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
>   bool ref)
>  {
>  virDomainObjPtr obj;
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return NULL;
>  obj = virHashSearch(doms->objs, virDomainObjListSearchID, , NULL);
>  if (ref) {
>  virObjectRef(obj);
> @@ -160,7 +161,8 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr 
> doms,
>  char uuidstr[VIR_UUID_STRING_BUFLEN];
>  virDomainObjPtr obj;
>  
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return NULL;
>  virUUIDFormat(uuid, uuidstr);
>  
>  obj = virHashLookup(doms->objs, uuidstr);
> @@ -204,7 +206,8 @@ virDomainObjPtr 
> virDomainObjListFindByName(virDomainObjListPtr doms,
>  {
>  virDomainObjPtr obj;
>  
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return NULL;
>  obj = virHashLookup(doms->objsName, name);
>  virObjectRef(obj);
>  virObjectUnlock(doms);
> @@ -653,7 +656,8 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms,
>   virConnectPtr conn)
>  {
>  struct virDomainObjListData data = { filter, conn, active, 0 };
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return -1;
>  virHashForEach(doms->objs, virDomainObjListCount, );
>  virObjectUnlock(doms);
>  return data.count;
> @@ -697,7 +701,8 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
>  {
>  struct virDomainIDData data = { filter, conn,
>  0, maxids, ids };
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return -1;
>  virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, );
>  virObjectUnlock(doms);
>  return data.numids;
> @@ -751,7 +756,8 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
>  struct virDomainNameData data = { filter, conn,
>0, 0, maxnames, names };
>  size_t i;
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return -1;
>  virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, );
>  virObjectUnlock(doms);
>  if (data.oom) {
> @@ -792,7 +798,8 @@ virDomainObjListForEach(virDomainObjListPtr doms,
>  struct virDomainListIterData data = {
>  callback, opaque, 0,
>  };
> -virObjectLockRead(doms);
> +if (virObjectLockRead(doms) < 0)
> +return -1;
>  virHashForEach(doms->objs, virDomainObjListHelper, );
>  virObjectUnlock(doms);
>  return data.ret;
> @@ -925,7 +932,8 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
>  {
>  struct virDomainListData data = { NULL, 0 };
>  
> -virObjectLockRead(domlist);
> +if (virObjectLockRead(domlist) < 0)
> +return -1;
>  sa_assert(domlist->objs);
>  if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
>  virObjectUnlock(domlist);
> @@ -962,7 +970,8 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
>  *nvms = 0;
>  *vms = NULL;
>  
> -virObjectLockRead(domlist);
> +if (virObjectLockRead(domlist) < 0)
> +return -1;
>  for (i = 0; i < ndoms; i++) {
>  virDomainPtr dom = doms[i];
>  
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index b1bb378..73de4d3 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -410,17 +410,21 @@ virObjectLock(void *anyobj)
>   * The object must be unlocked before releasing this
>   * reference.
>   */
> -void
> +int

I'm NACK on this return value change.

>  virObjectLockRead(void *anyobj)
>  {
>  if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
>  virObjectRWLockablePtr obj = anyobj;
>  virRWLockRead(>lock);
> +return 0;
>  } else {
>  virObjectPtr obj = anyobj;
>  VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
>   anyobj, obj ? obj->klass->name : "(unknown)");
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("unable to obtain rwlock for 

Re: [libvirt] [PATCH 0/7] Misc improvements

[Daniel P. Berrange]

On Mon, Jul 31, 2017 at 08:58:41AM +0200, Martin Kletzander wrote:
> On Fri, Jul 28, 2017 at 04:58:57PM +0100, Daniel P. Berrange wrote:
> > On Fri, Jul 28, 2017 at 11:47:56AM -0400, John Ferlan wrote:
> > > 
> > > 
> > > On 07/28/2017 11:24 AM, Daniel P. Berrange wrote:
> > > > On Fri, Jul 28, 2017 at 11:09:03AM -0400, John Ferlan wrote:
> > > >>
> > > >>
> > > >> On 07/28/2017 10:32 AM, Martin Kletzander wrote:
> > > >>> On Thu, Jul 27, 2017 at 01:47:20PM +0200, Michal Privoznik wrote:
> > > >>>> As I started to turn more object into using RW locks, I've found
> > > >>>> couple of
> > > >>>> areas for improvement too.
> > > >>>>
> > > >>>> Michal Privoznik (7):
> > > >>>>  virConnect: Update comment for @privateData
> > > >>>>  Report error if virMutexInit fails
> > > >>>>  virnetworkobj: Make virNetworkObjFindBy{UUID,Name}Locked() static
> > > >>>>again
> > > >>>>  virNetworkObjList: Derive from virObjectRWLockable
> > > >>>>  virNodeDeviceObjList: Derive from virObjectRWLockable
> > > >>>>  virConnect: Derive from virObjectRWLockable
> > > >>>>  storageDriver: Use RW locks
> > > >>>>
> > > >>>
> > > >>> The patches I have not replied to look fine, but I think it would be
> > > >>> easier to modify the common object after John's patches.  Are any of
> > > >>> those non-conflicting with those series?  If yes, I can review those
> > > >>> into more detail.
> > > >>>
> > > >>
> > > >> I had contacted Michal via IRC about this when I saw these hit the 
> > > >> list.
> > > >> I'd prefer to see them handled via a common object set of patches.
> > > >>
> > > >> However, that said... I wish the RWLockable hadn't just gone in so
> > > >> quickly, but what's done is done. I have a couple of other thoughts in
> > > >> this area:
> > > >>
> > > >>  * I think virObjectLockableRead should return 0/-1 and have the caller
> > > >> handle it.
> > > >>  * I think there should be a virObjectLockableWrite w/ same return 
> > > >> value
> > > >> checking.
> > > >
> > > > I rather disagree with that - it just adds a massive amount more
> > > > code to deal with failures from the lock apis that should never
> > > > happen unless you've already screwed up somewhere else in your
> > > > code. If the object you've passed into the methods has already
> > > > been freed, then you're already doomed and trying to recover from
> > > > that is never going to be reliable - in fact it could cause more
> > > > trouble. The memory for the object passed in is either in the free
> > > > pool (and so shouldn't be touched at all), or has been reused and
> > > > allocated for some other object now (and so again touching it is
> > > > a bad idea). Trying to detect & handle these situatuons is just
> > > > doomed to be racy or dangerous or both
> > > >
> > > 
> > > I agree w/ the screw up part. Obviously for me it's the RW vs non-RW
> > > usage that sent me down this path...
> > > 
> > > Still, I'm not sure what you mean by massive amount of code to deal with
> > > failures. I was considering only the RW lock mgmt.  Currently only
> > > virdomainobjlist was modified to add virObjectLockRead and only done
> > > within the last week. There's 9 virObjectLockRead calls and would be 4
> > > virObjectLockWrite calls.
> > > 
> > > if (virObjectLock{Read|Write}(obj) < 0)
> > > {goto {cleanup|error}|return -1|return NULL};
> > 
> > That's probably buggy if you use existing goto's, because many of
> > those cleanup/error locations will call virObjectUnlock(obj), so
> > you'll need to introduce another set of gotoo labels to optionally
> > skip the unlock step. This is why I think it makes the code more
> > complex for dubious benefit.
> > 
> > > The only place this doesn't work properly is the vir*Remove() calls
> > > which are void functions. We'd still be "stuck" with them.
> > 
> > Yes that's another scenario I imagined - there are case where it simply
> &

Re: [libvirt] [PATCH 0/7] Misc improvements

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 11:47:56AM -0400, John Ferlan wrote:
> 
> 
> On 07/28/2017 11:24 AM, Daniel P. Berrange wrote:
> > On Fri, Jul 28, 2017 at 11:09:03AM -0400, John Ferlan wrote:
> >>
> >>
> >> On 07/28/2017 10:32 AM, Martin Kletzander wrote:
> >>> On Thu, Jul 27, 2017 at 01:47:20PM +0200, Michal Privoznik wrote:
> >>>> As I started to turn more object into using RW locks, I've found
> >>>> couple of
> >>>> areas for improvement too.
> >>>>
> >>>> Michal Privoznik (7):
> >>>>  virConnect: Update comment for @privateData
> >>>>  Report error if virMutexInit fails
> >>>>  virnetworkobj: Make virNetworkObjFindBy{UUID,Name}Locked() static
> >>>>again
> >>>>  virNetworkObjList: Derive from virObjectRWLockable
> >>>>  virNodeDeviceObjList: Derive from virObjectRWLockable
> >>>>  virConnect: Derive from virObjectRWLockable
> >>>>  storageDriver: Use RW locks
> >>>>
> >>>
> >>> The patches I have not replied to look fine, but I think it would be
> >>> easier to modify the common object after John's patches.  Are any of
> >>> those non-conflicting with those series?  If yes, I can review those
> >>> into more detail.
> >>>
> >>
> >> I had contacted Michal via IRC about this when I saw these hit the list.
> >> I'd prefer to see them handled via a common object set of patches.
> >>
> >> However, that said... I wish the RWLockable hadn't just gone in so
> >> quickly, but what's done is done. I have a couple of other thoughts in
> >> this area:
> >>
> >>  * I think virObjectLockableRead should return 0/-1 and have the caller
> >> handle it.
> >>  * I think there should be a virObjectLockableWrite w/ same return value
> >> checking.
> > 
> > I rather disagree with that - it just adds a massive amount more
> > code to deal with failures from the lock apis that should never
> > happen unless you've already screwed up somewhere else in your
> > code. If the object you've passed into the methods has already
> > been freed, then you're already doomed and trying to recover from
> > that is never going to be reliable - in fact it could cause more
> > trouble. The memory for the object passed in is either in the free
> > pool (and so shouldn't be touched at all), or has been reused and
> > allocated for some other object now (and so again touching it is
> > a bad idea). Trying to detect & handle these situatuons is just
> > doomed to be racy or dangerous or both
> > 
> 
> I agree w/ the screw up part. Obviously for me it's the RW vs non-RW
> usage that sent me down this path...
> 
> Still, I'm not sure what you mean by massive amount of code to deal with
> failures. I was considering only the RW lock mgmt.  Currently only
> virdomainobjlist was modified to add virObjectLockRead and only done
> within the last week. There's 9 virObjectLockRead calls and would be 4
> virObjectLockWrite calls.
> 
> if (virObjectLock{Read|Write}(obj) < 0)
> {goto {cleanup|error}|return -1|return NULL};

That's probably buggy if you use existing goto's, because many of
those cleanup/error locations will call virObjectUnlock(obj), so
you'll need to introduce another set of gotoo labels to optionally
skip the unlock step. This is why I think it makes the code more
complex for dubious benefit.

> The only place this doesn't work properly is the vir*Remove() calls
> which are void functions. We'd still be "stuck" with them.

Yes that's another scenario I imagined - there are case where it simply
isn't practical to do cleanup when locking fails.

> Well I can propose the abort() on error if so desired. I agree w/r/t
> some awful things that could happen...

If we separate  virObjectLock vs virObjectRWLockWrite() then, we can
just unconditionally reference the object in the virObjectLock method
and just let the abort happen naturally, without needing explicit abort


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 0/7] Misc improvements

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 11:09:03AM -0400, John Ferlan wrote:
> 
> 
> On 07/28/2017 10:32 AM, Martin Kletzander wrote:
> > On Thu, Jul 27, 2017 at 01:47:20PM +0200, Michal Privoznik wrote:
> >> As I started to turn more object into using RW locks, I've found
> >> couple of
> >> areas for improvement too.
> >>
> >> Michal Privoznik (7):
> >>  virConnect: Update comment for @privateData
> >>  Report error if virMutexInit fails
> >>  virnetworkobj: Make virNetworkObjFindBy{UUID,Name}Locked() static
> >>again
> >>  virNetworkObjList: Derive from virObjectRWLockable
> >>  virNodeDeviceObjList: Derive from virObjectRWLockable
> >>  virConnect: Derive from virObjectRWLockable
> >>  storageDriver: Use RW locks
> >>
> > 
> > The patches I have not replied to look fine, but I think it would be
> > easier to modify the common object after John's patches.  Are any of
> > those non-conflicting with those series?  If yes, I can review those
> > into more detail.
> > 
> 
> I had contacted Michal via IRC about this when I saw these hit the list.
> I'd prefer to see them handled via a common object set of patches.
> 
> However, that said... I wish the RWLockable hadn't just gone in so
> quickly, but what's done is done. I have a couple of other thoughts in
> this area:
> 
>  * I think virObjectLockableRead should return 0/-1 and have the caller
> handle it.
>  * I think there should be a virObjectLockableWrite w/ same return value
> checking.

I rather disagree with that - it just adds a massive amount more
code to deal with failures from the lock apis that should never
happen unless you've already screwed up somewhere else in your
code. If the object you've passed into the methods has already
been freed, then you're already doomed and trying to recover from
that is never going to be reliable - in fact it could cause more
trouble. The memory for the object passed in is either in the free
pool (and so shouldn't be touched at all), or has been reused and
allocated for some other object now (and so again touching it is
a bad idea). Trying to detect & handle these situatuons is just
doomed to be racy or dangerous or both

>  * I think virObjectLock should not handle either RWLockable or
> Lockable. It should just handle Lockable.

I think that made sense as an intermediate step, to avoid having
to bulk-convert all code at once, but agree that it would be
reasonable to add a virObjectRWLockWrite() method, convert code
over, and then finally remove the code stuff in virObjectLock

>  * There could be a virObjectRWUnlock, but virObjectUnlock should be
> "OK" to not be specific since theoretically one would have already
> locked and got something valid. I think through this common object
> series I've found a few instances where Unlock was called with an
> Unlocked object which is a different can-o-worms. I have not come across
> any instance where Unlock was called with NULL or invalid parameter. And
> if it was, the worse thing that could happen is we wouldn't unlock the
> resource and that'd be found relatively quickly by the next locker.
> Debugging it would be a beachball though.

I think it would make sense to have a virObjectRWUnlock too.

Both of these changes would make it clearer which bit of code is
using a plain lockable object, vs a rwlockable object.

> FWIW: As noted in my responses to the RWLock series, consider if
> virObjectLock(obj) is called with an invalid @obj, then we really don't
> get the lock. All that's done is a VIR_WARN() and return. So if someone
> passes the wrong thing we have all sorts of problems. That's been true
> of virObjectLock for a long time, but we have (ahem) well behaved code
> so less of a problem.

As above, trying to detect these errors & carry on & cleanup is
just giving a false sense of security. I think this is probably a
case where it is reasonable to abort() immediately, because if you
are in this messed up state, the chances are that the code is going
to abort due to memory corrupton shortly anyway.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] Exposing mem-path in domain XML

[Daniel P. Berrange]

On Fri, Jul 28, 2017 at 10:45:21AM +0200, Michal Privoznik wrote:
> On 07/27/2017 03:50 PM, Daniel P. Berrange wrote:
> > On Thu, Jul 27, 2017 at 02:11:25PM +0200, Michal Privoznik wrote:
> >> Dear list,
> >>
> >> there is the following bug [1] which I'm not quite sure how to grasp. So
> >> there is this application/infrastructure called Kove [2] that allows you
> >> to have memory for your application stored on a distant host in network
> >> and basically fetch needed region on pagefault. Now imagine that
> >> somebody wants to use it for backing up domain memory. However, the way
> >> that the tool works is it has some kernel module and then some userland
> >> binary that is fed with the path of the mmaped file. I don't know all
> >> the details, but the point is, in order to let users use this we need to
> >> expose the paths for mem-path for the guest memory. I know we did not
> >> want to do this in the past, but now it looks like we don't have a way
> >> around it, do we?
> > 
> > We don't want to expose the concept of paths in the XML because this is
> > a linux specific way to configure hugepages / shared memory. So we hide
> > the particular path used in the internal impl of the QEMU driver, and
> > or via the qemu.conf global config file. I don't really want to change
> > that approach, particularly if the only reason is to integrate with a
> > closed source binary like Kove. 
> 
> Yep, I agree with that. However, if you read the discussion in the
> linked bug you'll find that they need to know what file in the
> memory_backing_dir (from qemu.conf) corresponds to which domain. The
> reported suggested using UUID based filenames, which I fear is not
> enough because one can have multiple  -s configured
> for their domain. But I guess we could go with:
> 
> ${memory_backing_dir}/${domName}for generic memory
> ${memory_backing_dir}/${domName}_N  for Nth 

This feels like it is going to lead to hell when you add in memory
hotplug/unplug, with inevitable races.

> BTW: IIUC they want predictable names because they need to create the
> files before spawning qemu so that they are picked by qemu instead of
> using temporary names.

I would like to know why they even need to associate particular memory
files with particular QEMU processes. eg if they're just exposing a
new type of tmpfs filesystem from the kernel why does it matter what
each file is used for.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Daniel P. Berrange]

On Thu, Jul 27, 2017 at 12:01:58PM -0600, Alex Williamson wrote:
> On Thu, 27 Jul 2017 17:17:48 +0100
> "Daniel P. Berrange" <berra...@redhat.com> wrote:
> 
> > On Wed, Jul 26, 2017 at 10:43:43AM -0600, Alex Williamson wrote:
> > > [cc +libvir-list]
> > > 
> > > On Wed, 26 Jul 2017 21:16:59 +0800
> > > "Gao, Ping A" <ping.a@intel.com> wrote:
> > >   
> > > > The vfio-mdev provide the capability to let different guest share the
> > > > same physical device through mediate sharing, as result it bring a
> > > > requirement about how to control the device sharing, we need a QoS
> > > > related interface for mdev to management virtual device resource.
> > > > 
> > > > E.g. In practical use, vGPUs assigned to different quests almost has
> > > > different performance requirements, some guests may need higher priority
> > > > for real time usage, some other may need more portion of the GPU
> > > > resource to get higher 3D performance, corresponding we can define some
> > > > interfaces like weight/cap for overall budget control, priority for
> > > > single submission control.
> > > > 
> > > > So I suggest to add some common attributes which are vendor agnostic in
> > > > mdev core sysfs for QoS purpose.  
> > > 
> > > I think what you're asking for is just some standardization of a QoS
> > > attribute_group which a vendor can optionally include within the
> > > existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
> > > transparently enable this, but it really only provides the standard,
> > > all of the support code is left for the vendor.  I'm fine with that,
> > > but of course the trouble with and sort of standardization is arriving
> > > at an agreed upon standard.  Are there QoS knobs that are generic
> > > across any mdev device type?  Are there others that are more specific
> > > to vGPU?  Are there existing examples of this that we can steal their
> > > specification?
> > > 
> > > Also, mdev devices are not necessarily the exclusive users of the
> > > hardware, we can have a native user such as a local X client.  They're
> > > not an mdev user, so we can't support them via the mdev_attr_group.
> > > Does there need to be a per mdev parent QoS attribute_group standard
> > > for somehow defining the QoS of all the child mdev devices, or perhaps
> > > representing the remaining host QoS attributes?
> > > 
> > > Ultimately libvirt and upper level management tools would be the
> > > consumer of these control knobs, so let's immediately get libvirt
> > > involved in the discussion.  Thanks,  
> > 
> > My view on this from libvirt side is pretty much unchanged since the
> > last time we discussed this.
> > 
> > We would like the kernel maintainers to define standard sets of properties
> > for mdevs, whether global to all mdevs, or scoped to certain classes of
> > mdev (eg a class=gpu). These properties would be exported in sysfs, with
> > one file per property.
> 
> Yes, I think that much of the mechanics are obvious (standardized
> sysfs layout, one property per file, properties under the device node
> in sysfs, etc).  Are you saying that you don't want to be consulted on
> which properties are exposed and how they operate and therefore won't
> complain regardless of what we implement in the kernel? ;)

Well ultimately the kernel maintainers know what is possible from the
hardware / driver POV, so yeah, I think we can mostly leave it upto
you what individual things need to be exposed - not much different
scenario from all the knobs we already exposed for physical devices.


> I'm hoping that libvirt folks have some experience managing basic
> scheduling level QoS attributes and might have some input as to what
> sorts of things work well vs what seems like a good idea, but falls
> apart or isn't useful in practice.

Sure, happy to give feedback where desired.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] Entering freeze for libvirt-3.6.0

[Daniel P. Berrange]

On Thu, Jul 27, 2017 at 06:14:58PM +0200, Andrea Bolognani wrote:
> On Thu, 2017-07-27 at 22:42 +0800, Daniel Veillard wrote:
> >   As planned I tagged 3.6.0-rc1 in git and I made the Release Candidate 1
> > signed tarball and rpms available from the usual place:
> > 
> >   ftp://libvirt.org/libvirt/
> > 
> > 
> > seems to work in my limited testing, https://ci.centos.org/view/libvirt/
> > is all green (except for mingw maybe there is a portability issue)
> 
> That's because after 5aec02dc3762 dlopen() and friends are
> required for building, and mingw doesn't provide them.

No that's not correct - dlopen() is only required if building stateful
drivers with libvirtd enabled, and that's not needed with mingw. The
biuld failure is actually simpler - its just a test case that's not
compiling

../../tests/virdrivermoduletest.c: In function 'mymain':
../../tests/virdrivermoduletest.c:55:33: error: unused variable 'data' 
[-Werror=unused-variable]
 struct testDriverModuleData data;
 ^~~~
At top level:
../../tests/virdrivermoduletest.c:39:12: error: 'testDriverModule' defined but 
not used [-Werror=unused-function]
 static int testDriverModule(const void *args)
^~~~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:5331: virdrivermoduletest.o] Error 1


We simply need to conditionalize that test properly

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Daniel P. Berrange]

On Wed, Jul 26, 2017 at 10:43:43AM -0600, Alex Williamson wrote:
> [cc +libvir-list]
> 
> On Wed, 26 Jul 2017 21:16:59 +0800
> "Gao, Ping A"  wrote:
> 
> > The vfio-mdev provide the capability to let different guest share the
> > same physical device through mediate sharing, as result it bring a
> > requirement about how to control the device sharing, we need a QoS
> > related interface for mdev to management virtual device resource.
> > 
> > E.g. In practical use, vGPUs assigned to different quests almost has
> > different performance requirements, some guests may need higher priority
> > for real time usage, some other may need more portion of the GPU
> > resource to get higher 3D performance, corresponding we can define some
> > interfaces like weight/cap for overall budget control, priority for
> > single submission control.
> > 
> > So I suggest to add some common attributes which are vendor agnostic in
> > mdev core sysfs for QoS purpose.
> 
> I think what you're asking for is just some standardization of a QoS
> attribute_group which a vendor can optionally include within the
> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
> transparently enable this, but it really only provides the standard,
> all of the support code is left for the vendor.  I'm fine with that,
> but of course the trouble with and sort of standardization is arriving
> at an agreed upon standard.  Are there QoS knobs that are generic
> across any mdev device type?  Are there others that are more specific
> to vGPU?  Are there existing examples of this that we can steal their
> specification?
> 
> Also, mdev devices are not necessarily the exclusive users of the
> hardware, we can have a native user such as a local X client.  They're
> not an mdev user, so we can't support them via the mdev_attr_group.
> Does there need to be a per mdev parent QoS attribute_group standard
> for somehow defining the QoS of all the child mdev devices, or perhaps
> representing the remaining host QoS attributes?
> 
> Ultimately libvirt and upper level management tools would be the
> consumer of these control knobs, so let's immediately get libvirt
> involved in the discussion.  Thanks,

My view on this from libvirt side is pretty much unchanged since the
last time we discussed this.

We would like the kernel maintainers to define standard sets of properties
for mdevs, whether global to all mdevs, or scoped to certain classes of
mdev (eg a class=gpu). These properties would be exported in sysfs, with
one file per property.

Libvirt can then explicitly map each standardized property into a suitable
XML element, to report on which properties are available to use when creating
an mdev. It would then allow them to be set at time of creation, and/or
changed on the fly for existing mdevs.

Specifically we would like to avoid generic passthrough of arbitrary
vendor specific properties.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 00/12] Cleanup website generation & add favicons

[Daniel P. Berrange]

On Thu, Jul 27, 2017 at 04:02:35PM +0200, Michal Privoznik wrote:
> On 07/26/2017 07:51 PM, Daniel P. Berrange wrote:
> > This started as an attempt to add modern favicon support to
> > the website. This requires use of HTML5 only syntax, which
> > lead to the massive cleanup to stop using XHTML 1.0, which
> > forms all of this series except the last patch
> > 
> > Daniel P. Berrange (12):
> >   docs: switch to using 'id' attribute instead of 'name' for links
> >   docs: drop XHTML 1.0 validation of website
> >   docs: make xmllint & xsltproc compulsory
> >   docs: fix typo s///
> >   docs: remove use of  in docs
> >   docs: remove use of  entity
> >   docs: use UTF-8 instead of HTML entities for decorated letters
> >   docs: switch to using HTML5 doctype declaration
> >   docs: generate pretty indented HTML for API docs
> >   docs: remove bogus 'shape' attribute on links
> >   docs: explicitly declare pages as being UTF-8 format
> >   docs: add full set of "favicon" files to support modern clients
> > 
> >  .travis.yml  |   1 -
> >  docs/404.html.in |   2 +-
> >  docs/Makefile.am |  52 +--
> >  docs/acl.html.in |   8 +-
> >  docs/aclpolkit.html.in   |  34 +-
> >  docs/android-chrome-192x192.png  | Bin 0 -> 13057 bytes
> >  docs/android-chrome-256x256.png  | Bin 0 -> 16597 bytes
> >  docs/api.html.in |  10 +-
> >  docs/api_extension.html.in   |  14 +-
> >  docs/apple-touch-icon.png| Bin 0 -> 10489 bytes
> >  docs/apps.html.in|  38 +--
> >  docs/architecture.html.in|   8 +-
> >  docs/auditlog.html.in|  40 +--
> >  docs/auth.html.in|  16 +-
> >  docs/bindings.html.in|   2 +-
> >  docs/browserconfig.xml   |   9 +
> >  docs/bugs.html.in|  12 +-
> >  docs/cgroups.html.in |  26 +-
> >  docs/compiling.html.in   |   8 +-
> >  docs/contact.html.in |   8 +-
> >  docs/contribute.html.in  |  12 +-
> >  docs/csharp.html.in  | 634 
> > +--
> >  docs/devguide.html.in|   2 +-
> >  docs/docs.html.in|   2 +-
> >  docs/downloads.html.in   |  14 +-
> >  docs/drivers.html.in |   6 +-
> >  docs/drvbhyve.html.in|  22 +-
> >  docs/drvesx.html.in  |  46 +--
> >  docs/drvhyperv.html.in   |  12 +-
> >  docs/drvlxc.html.in  |  54 +--
> >  docs/drvnodedev.html.in  |  10 +-
> >  docs/drvopenvz.html.in   |  14 +-
> >  docs/drvphyp.html.in |   8 +-
> >  docs/drvqemu.html.in |  34 +-
> >  docs/drvremote.html.in   |   2 +-
> >  docs/drvtest.html.in |   2 +-
> >  docs/drvuml.html.in  |   4 +-
> >  docs/drvvbox.html.in |   6 +-
> >  docs/drvvirtuozzo.html.in|   8 +-
> >  docs/drvvmware.html.in   |   6 +-
> >  docs/drvxen.html.in  |  16 +-
> >  docs/errors.html.in  |   2 +-
> >  docs/favicon-16x16.png   | Bin 0 -> 872 bytes
> >  docs/favicon-32x32.png   | Bin 0 -> 1793 bytes
> >  docs/favicon.ico | Bin 0 -> 15086 bytes
> >  docs/firewall.html.in|   8 +-
> >  docs/format.html.in  |  22 +-
> >  docs/formatcaps.html.in  |  10 +-
> >  docs/formatdomain.html.in| 228 ++---
> >  docs/formatdomaincaps.html.in|  26 +-
> >  docs/formatnetwork.html.in   |  36 +-
> >  docs/formatnode.html.in  |   6 +-
> >  docs/formatnwfilter.html.in  |  70 ++--
> >  docs/formatsecret.html.in|  12 +-
> >  docs/formatsnapshot.html.in  |   6 +-
> >  docs/formatstorage.html.in   |  30 +-
> >  docs/formatstorageencryption.html.in |  12 +-
> >  docs/genaclperms.pl  |   2 +-
> >  docs/goals.html.in   |   2 +-
> >  docs/governance.html.in  |  14 +-
> >  docs/hacking.html.in |  42 +--
> >  docs/hooks.html.in   |  34 +-
> >  docs/hvsupport.pl|   2 +-
> >  docs/index.html.in   |  22 +-
> >  docs/internals.html.i

Re: [libvirt] Exposing mem-path in domain XML

[Daniel P. Berrange]

On Thu, Jul 27, 2017 at 02:11:25PM +0200, Michal Privoznik wrote:
> Dear list,
> 
> there is the following bug [1] which I'm not quite sure how to grasp. So
> there is this application/infrastructure called Kove [2] that allows you
> to have memory for your application stored on a distant host in network
> and basically fetch needed region on pagefault. Now imagine that
> somebody wants to use it for backing up domain memory. However, the way
> that the tool works is it has some kernel module and then some userland
> binary that is fed with the path of the mmaped file. I don't know all
> the details, but the point is, in order to let users use this we need to
> expose the paths for mem-path for the guest memory. I know we did not
> want to do this in the past, but now it looks like we don't have a way
> around it, do we?

We don't want to expose the concept of paths in the XML because this is
a linux specific way to configure hugepages / shared memory. So we hide
the particular path used in the internal impl of the QEMU driver, and
or via the qemu.conf global config file. I don't really want to change
that approach, particularly if the only reason is to integrate with a
closed source binary like Kove. 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] tests: add virfilecachedata to EXTRA_DIST

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed as a build fix

 tests/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8349bbec4..9a822f7d5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ EXTRA_DIST =\
xml2sexprdata \
xml2vmxdata \
virstorageutildata \
+   virfilecachedata \
$(NULL)
 
 test_helpers = commandhelper ssh
-- 
2.13.3

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

Re: [libvirt] [PATCH 2/2] security: apparmor: load the storage driver dynamically

[Daniel P. Berrange]

On Wed, Jul 26, 2017 at 10:12:14PM +0200, Peter Krempa wrote:
> In commit 5e515b542d I've attempted to fix the inability to access
> storage from the apparmor helper program by linking with the storage
> driver. By linking with the .so the linker complains that it's not
> portable. Fix this by loading the module dynamically as we are supposed
> to do.
> ---
> 
> Notes:
> This patch is possible even with the previous patch, but it would be 
> slightly
> more complex, since it would need the logic to determine whether to load 
> the
> module or just initialize it.
> 
>  src/Makefile.am   |  2 +-
>  src/security/virt-aa-helper.c | 10 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH RFC 1/2] make: Drop building without driver modules

[Daniel P. Berrange]

On Wed, Jul 26, 2017 at 10:12:13PM +0200, Peter Krempa wrote:
> Driver modules proved to be reliable for a long time. Since support for
> not building modules complicates the code and makefiles drop the support
> for not building drivers as modules.
> ---
> 
> Notes:
> This was suggested a while ago by Dan:
> 
> https://www.redhat.com/archives/libvir-list/2017-March/msg00917.html
> 
> I actually did not try to build this on windows, since I don't have the
> environment ready (do we actually even build the daemon on windows?).

We don't build the daemon on Win32. If we did ever want todo that, Win32
has an equivalent to dlopen we could use, so modules aren't a blocker for
that.

> daemon/Makefile.am|  57 ---
> daemon/libvirtd.c |  54 +--
> m4/virt-driver-modules.m4 |  24 +++
> src/Makefile.am   | 158 --
> src/driver.c  |   8 +--
> src/storage/storage_backend.c |  11 +--
> src/vbox/vbox_driver.c|   2 +-
> src/vbox/vbox_driver.h|   6 +-
> tests/Makefile.am |   6 --
> tests/testutils.c |   2 -
> tools/virsh.c |   3 -
> 11 files changed, 21 insertions(+), 310 deletions(-)
>

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH 06/12] docs: remove use of entity

[Daniel P. Berrange]

A handful of places in the docs choose to use  instead
of '-' for no clear reason. Remove this inconsistency.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/formatdomain.html.in  | 34 +-
 docs/formatnetwork.html.in |  2 +-
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb22dd1db..0ed12ecb0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6914,10 +6914,10 @@ qemu-kvm -net nic,model=? /dev/null
 QEMU and KVM support:
 
 
-   'i6300esb'  the recommended device,
+  'i6300esb' - the recommended device,
 emulating a PCI Intel 6300ESB 
-   'ib700'  emulating an ISA iBase IB700 
-   'diag288'  emulating an S390 DIAG288 device
+  'ib700' - emulating an ISA iBase IB700 
+  'diag288' - emulating an S390 DIAG288 device
 Since 1.2.17
 
   
@@ -6932,15 +6932,15 @@ qemu-kvm -net nic,model=? /dev/null
 QEMU and KVM support:
 
 
-  'reset'  default, forcefully reset the guest
-  'shutdown'  gracefully shutdown the guest
+  'reset' - default, forcefully reset the guest
+  'shutdown' - gracefully shutdown the guest
 (not recommended) 
-  'poweroff'  forcefully power off the guest
-  'pause'  pause the guest
-  'none'  do nothing
-  'dump'  automatically dump the guest
+  'poweroff' - forcefully power off the guest
+  'pause' - pause the guest
+  'none' - do nothing
+  'dump' - automatically dump the guest
 Since 0.8.7
-  'inject-nmi'  inject a non-maskable interrupt
+  'inject-nmi' - inject a non-maskable interrupt
 into the guest
 Since 1.2.17
 
@@ -7005,8 +7005,8 @@ qemu-kvm -net nic,model=? /dev/null
   the virtualization platform
 
 
-  'virtio'  default with QEMU/KVM
-  'xen'  default with Xen
+  'virtio' - default with QEMU/KVM
+  'xen' - default with Xen
 
   
   autodeflate
@@ -7078,7 +7078,7 @@ qemu-kvm -net nic,model=? /dev/null
   the virtualization platform:
 
 
-  'virtio'  supported by qemu and virtio-rng kernel 
module
+  'virtio' - supported by qemu and virtio-rng kernel module
 
   
   rate
@@ -7270,11 +7270,11 @@ qemu-kvm -net nic,model=? /dev/null
   is missing depends on the hypervisor and guest arch.
 
 
-  'isa'  for ISA pvpanic device
-  'pseries'  default and valid only for pSeries guests.
-  'hyperv'  for Hyper-V crash CPU feature.
+  'isa' - for ISA pvpanic device
+  'pseries' - default and valid only for pSeries guests.
+  'hyperv' - for Hyper-V crash CPU feature.
 Since 1.3.0, QEMU and KVM only
-  's390'  default for S390 guests.
+  's390' - default for S390 guests.
 Since 1.3.5
 
   
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index e8e618e42..9c07e5e24 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -997,7 +997,7 @@
 prefix attribute, which is an integer (for example,
 netmask='255.255.255.0' could also be given as
 prefix='24'). The family attribute is used
-to specify the type of address  ipv4 or
+to specify the type of address - ipv4 or
 ipv6; if no family is given,
 ipv4 is assumed. More than one address of each family can
 be defined for a network. The optional localPtr attribute
-- 
2.13.3

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

[libvirt] [PATCH 12/12] docs: add full set of "favicon" files to support modern clients

[Daniel P. Berrange]

Use of the relation "shortcut" for a favicon was an Internet
Explorer only feature. Other browsers just require "icon".

The new icons & metadata are generated using

  https://realfavicongenerator.net/

which is user tested to work well across all modern clients

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/android-chrome-192x192.png | Bin 0 -> 13057 bytes
 docs/android-chrome-256x256.png | Bin 0 -> 16597 bytes
 docs/apple-touch-icon.png   | Bin 0 -> 10489 bytes
 docs/browserconfig.xml  |   9 +
 docs/favicon-16x16.png  | Bin 0 -> 872 bytes
 docs/favicon-32x32.png  | Bin 0 -> 1793 bytes
 docs/favicon.ico| Bin 0 -> 15086 bytes
 docs/manifest.json  |  18 ++
 docs/mstile-150x150.png | Bin 0 -> 7579 bytes
 docs/page.xsl   |   6 +-
 10 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 docs/android-chrome-192x192.png
 create mode 100644 docs/android-chrome-256x256.png
 create mode 100644 docs/apple-touch-icon.png
 create mode 100644 docs/browserconfig.xml
 create mode 100644 docs/favicon-16x16.png
 create mode 100644 docs/favicon-32x32.png
 create mode 100644 docs/favicon.ico
 create mode 100644 docs/manifest.json
 create mode 100644 docs/mstile-150x150.png

diff --git a/docs/android-chrome-192x192.png b/docs/android-chrome-192x192.png
new file mode 100644
index 
..2edadf93f1f270a9ed4d2c01e9f71847078b8409
GIT binary patch
literal 13057
zcmX|n19W9gu<nU%Yhq38Ol;e>G2w~r6Wg|JYm%ARwmGr&=D{*K60VuCJ@wtJmJu
z1yWLwM1seM2LJ#_(o$k7|9Jm@1Pk%+Z4REz@sB}T3d;)v01a^n@5WI7WH1*MNfAKp
z6yfPV0nSlM+vT6}@_z&-LX+hI06+kg<kZCFtev2sprN5*{%QR)`bUsZ|C#?w;Nal@
zn*jp@gMj$2secOp(*Lsm5>HQ!0|1%kxnXz$k5Rg#r{-JaXOunJf
z*tmFJK_H-Ch^>>Gt8eg6cV9Lx9%HMY=623X8oK|Abd4?KRW$Gjh-8)3`Gmx+99(JX
znYelRr4?0q1VnIf@nsZM)N~9r^-b8hxXo=H*u0zohQ_{OF<)O_|N7$>9(!_f!s_V|
z2#N<7o3MMk_=HBYxY+~rjm)g=FV4@6%%nZ6Khbd3bodLn4Et6S;!DEbN?ZUEITB
zlNmki=`4-(jLluW0|C1FacSAy0Ul9F8EH9%U#9>t4Q<b$h@WoW`K476@u^N8ej;LG
zC6%?DzCd|JC0bKGG<5XL{F2YlPXH|~A|ir-pkPvF9snJKLRXF9r^VRBR8CRZ!QoLw
zT{9Rd>D${I7#=>ot(mn0P(@9BV|%BrsRc|$_21-NU*7=a<aPB8l9E#%pB{S#hFdy&
z)U|Y1H#WO^2e<_UclP#v{T??nGsEF#!@<ML%gbkFXD=x&8yp^)TUewv)cU3%BQ7OP
zrlADj<N`xL1TeEifIu>`GPYUC0CCCc>S}BuUVyMjV`~Q)IYm%#$kNIxH7%_ID9Ak^
zj8{x_YIdGLT9okHH_xIR0H1(|m$zuJw=2+%o{_PCda|Rl)85{m@&|`eatw!oHlL*_
zi;$pZT*MDq>ABTaKR-VVB_%7rVDEsCwm+jG6~$CyV%0>iHRa;oP7MzqKA^+c
z;{3)8L`GYafVzqRAJpk87@M!0PcA*Vvr=^jK|*8Auit2>HkvX@53;HWq>7S=oxZ
zc`AT}<Ep4ANWsjLx#s!+Qq~j(>VSkuNGEs1&_FD0WB9joF!Ypq=8uQ!
zuZQHHR%F|2u5mFA`-@n9Ws6n+Hj22bih?Qt7lQ~M0vQ|(hLp*S<2z-fJzs^eu7XZq!frKk8{_2Uye1HgzwmFkddkU7jpQ+i*a_p3
z=Pt3n<-2<8zIyX96nwK2d^G*PXa9>FUCF0%(fKO*kyUd!kLGz-AaLxz#!LNWNB
zGIB$j;>Su@|I=}9e{4)fR(5<=EocqEP9zaY<%xb8kqL-eg8Lz3OrFW~Y<CO@>
zEmZ*Togs|tYf{)=U0hFMSj_1O0#ZK3&|IrKS%pMl7_u(`UqU8vUn8!jQC
z(Cm6?nax<Bh;yb`vnJlohUsa9Y!t%X!(?#qav6@qTkU-eqn!Jst1eAr{&!xsANpjm
zvT_?Ph^Rh%uEXZk@CXa~5c@suZ>2WJOQfmcHf67WAKlLXyw^1$my}-eL!+GzlkVDP
zT8t2X@!7C5GWG0m#~S0BSo==dW<w3#X<{39oeen|^GIFk@56M07?`}AxyHOO%H
zIu~`vBeJNvJzr1=cl2v7+w<NFM_m;OF)=A0+PfG9@^ly^6Lois#PC7hMr^=3
z#a8+H=kf9J&(hK>GL&%nk}VSlQ29{399l{o2HlO=$IK!%6*HBf^HtKqnF;TBDb1Bf
zVRnAauZgI>ipT5h91ZH%UOa7C%=g!9OOj1{H1Q{OZE2^#PWyb9>tkvOdLo6{*+^%U
zdh1LQD=3x3APK~^X0wNeWb%sB!c7L@;j`<x*{Nt(1Apf~i1t+wyX2>Gt+`9ny
zbYXkLrtbIk{qUv)2-qgn}e34KMJSwrJPKEe^c2WiWnh0Vzv5FU;mbh_M2%pMFbf
zstHcabZqv^GWCmR0oS^($Hd36nK2~qcWAp+n68LE&^W>+N(<8&^kIcef{K3>HgnL2
zb%=oLJMU6c<;}wh(~oRP^H`Niy!nzIz@L
z-5~aQ282=MTXL1thO%|ZCQ{;P6F(U_HHzU^2xuMq~JZoCW5wXuk^fQq>RysgF
z7J8;S2IP5fot<RyT1q;<n$%2<4_f}%Y{ff!1P8+$<0vkOJ=ArUpT&_x`D;ahFiLw6
z`2xFrD7EiVHNWAI*Vl>K%A*@XjF9#S4S{SyM;c~C<o2zPe`g0bQ<-6j@#IRBAUKSI
zw+X(mOPwAYB!&)x+zgDERgcBhB^uv}?7`_cSk@*2>zgl9Tk{3?NOgkb_X}q$7JKFw
zAd1pp)fZeSOCv`j60S0Wfi;pu$-s`HIJLI^<5OHf0J(MMsyOkem?gyHP|suhn`U)M
zeIoQQdrCb^t^%=9Sl3F6O4n(=yX?f{r$$wj!ne%=b<{8>Y|0YKG7Ae$4Gnd5b
z1d?ACL4(fe<~k(!8Rq8Iw`pHfxP%^uL0HQ_e*J6tB
zRXY$IAt-79!^GU|%p7$fRb75JUtf1ONnK@S=Si=dqsiVG#{A4}R?SqZ<1;^F&`1%6
zt7(e1J>56z@kZNvc<`W_s=TfeO1kUs|Uwfq9vp#kX?80@BJ9@R6Nw%Ta5ApX(?C
zXJV6sRW%B7j)=!D;1D@5r+ywoa2N=mm!kN$p|XIg9e1Sap#5+*K}Rf-caq$DGe|*0
zQ>7%41wOssIR?!7S-N_WXaXM=&+JHZ4KQG=O_8vW8P_<jM$~3i^oHPCP)KXPzkB#2
zQC>e)wSHUmf#|)6*z$(QQsmA^NuvPxW;!p9;fFOJ9_-ZGk{reAkH42iV=7H!=
z(S4WpU<~^Lo4ahSgd15@8wPQrzp~10L3Qxri%rW&{xSg>B}pHfKo5)dcCd@!%jk(R
zjDuG-a+@K@X}%M{X9vq0_)sI{)EM^F=Vd;}%BZ#|*Y`ej5L|zla%USQQgl
zK6{_X%SEy!!-KmYrc!pT~UQa%2|xDavIc6eb9jHJxds*o?lQOrn0@QKmq$4
zr=EnlBMC|G^?eQsNz~NT{o96}?~fPu-9wYDCXWl$@3{duC2Kg}Du$4EyVBzjkyNV-
z%*m}z%tpdj;K2OH8qV+|lkcGFwaCZ{{n9(R+>Vc0@8?744%XqfFCC4)iq?i9aSuMZ
z^7T(igrBHrHx9?`pVo~89<(C4Px4xN8UTYJ3Ar%DkgA+-n@Igf+o
zm|K7Pw4jw7Y~AbDWa%XX+`hiWBVPv0;yF

[libvirt] [PATCH 04/12] docs: fix typo s///

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/hooks.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index 7a04ac198..6cc47a6c5 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -289,7 +289,7 @@
   Since 1.2.2, before a network is started,
 this script is called as:
   /etc/libvirt/hooks/network network_name start begin -
-  After the network is started, up  running, the script is
+  After the network is started, up  running, the script is
 called as:
   /etc/libvirt/hooks/network network_name started begin 
-
   When a network is shut down, this script is called as:
-- 
2.13.3

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

[libvirt] [PATCH 09/12] docs: generate pretty indented HTML for API docs

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/newapi.xsl | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index a55736d32..a2f0e0714 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -814,6 +814,7 @@
 
   
 
@@ -828,6 +829,7 @@
   
 
   
-- 
2.13.3

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

[libvirt] [PATCH 11/12] docs: explicitly declare pages as being UTF-8 format

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/page.xsl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/page.xsl b/docs/page.xsl
index 4d49be085..d9be66b93 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -83,6 +83,7 @@
 Do not edit this file. Changes will be lost.
   
   
+
 
 
 libvirt: 
-- 
2.13.3

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

[libvirt] [PATCH 03/12] docs: make xmllint & xsltproc compulsory

[Daniel P. Berrange]

We already require libxml to be installed, so it is not unreasonable
to require xmllint and xsltproc to be installed too - any platform
with the former will have the latter too.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/Makefile.am | 44 +++-
 m4/virt-external-programs.m4 | 12 ++--
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index f478d9505..d6c9d0091 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -221,17 +221,14 @@ $(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.pl 
$(api_DATA) \
 news.html.in: \
  $(srcdir)/news.xml \
  $(srcdir)/news-html.xsl
-   $(AM_V_GEN) \
-   if [ -x $(XSLTPROC) ]; then \
- $(XSLTPROC) --nonet \
+   $(AM_V_GEN)$(XSLTPROC) --nonet \
$(srcdir)/news-html.xsl \
$(srcdir)/news.xml \
  >$@-tmp \
|| { rm -f $@-tmp; exit 1; }; \
  sed 's/ xmlns=""//g' $@-tmp >$@ \
|| { rm -f $@-tmp; exit 1; }; \
- rm -f $@-tmp; \
-   fi
+ rm -f $@-tmp
 EXTRA_DIST += \
$(srcdir)/news.xml \
$(srcdir)/news.rng \
@@ -244,9 +241,7 @@ MAINTAINERCLEANFILES += \
 
 %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
$(acl_generated)
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Generating $@"; \
- name=`echo $@ | sed -e 's/.tmp//'`; \
+   $(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
  dir=`dirname $@` ; \
  if test "$$dir" = "."; \
  then \
@@ -257,42 +252,33 @@ MAINTAINERCLEANFILES += \
  fi; \
  $(XSLTPROC) --stringparam pagename $$name --nonet \
$(top_srcdir)/docs/$$style $< > $@ \
-   || { rm $@ && exit 1; }; fi
+   || { rm $@ && exit 1; }
 
 %.html: %.html.tmp
-   @if test -x $(XMLLINT) ; then \
- echo "Validating $@" ; \
- $(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
- || { rm $(srcdir)/$@ && exit 1; }; fi
+   $(AM_V_GEN)$(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
+ || { rm $(srcdir)/$@ && exit 1; }
 
 %.php.tmp: %.php.in site.xsl page.xsl
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Generating $@"; \
- $(XSLTPROC) --stringparam pagename $(@:.tmp=) --nonet \
+   $(AM_V_GEN)$(XSLTPROC) --stringparam pagename $(@:.tmp=) --nonet \
$(top_srcdir)/docs/site.xsl $< > $@ \
-   || { rm $@ && exit 1; }; fi
+   || { rm $@ && exit 1; }
 
 %.php: %.php.tmp %.php.code.in
-   @if [ -x $(XSLTPROC) ] ; then \
- echo "Scripting $@"; \
-   sed -e '/<\/span>/r 
'"$(srcdir)/$@.code.in" \
+   $(AM_V_GEN)sed -e '/<\/span>/r 
'"$(srcdir)/$@.code.in" \
-e /php_placeholder/d < $@.tmp > $(srcdir)/$@ \
-   || { rm $(srcdir)/$@ && exit 1; }; fi
+   || { rm $(srcdir)/$@ && exit 1; }
 
 $(apihtml_generated): html/index.html
 
 html/index.html: libvirt-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP)
-   $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
- $(XSLTPROC) --nonet -o $(srcdir)/ \
+   $(AM_V_GEN)$(XSLTPROC) --nonet -o $(srcdir)/ \
  --stringparam builddir '$(abs_top_builddir)' \
- $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi && \
-   if test -x $(XMLLINT) ; then \
- $(XMLLINT) --nonet --noout $(srcdir)/html/*.html ; fi
+ $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml && \
+ $(XMLLINT) --nonet --noout $(srcdir)/html/*.html
 
 $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
-   $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
- $(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \
- $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml ; fi
+   $(AM_V_GEN)$(XSLTPROC) --nonet -o $(srcdir)/devhelp/ \
+ $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml
 
 
 python_generated_files = \
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index 4a10c85ad..ab6149288 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -23,8 +23,16 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
   AM_CONDITIONAL([HAVE_RPCGEN], [test "x$ac_cv_path_RPCGEN" != "xno"])
 
   dnl Miscellaneous external programs.
-  AC_PATH_PROG([XMLLINT], [xmllint], [/usr/bin/xmllint])
-  AC_PATH_PROG([XSLTPROC], [xsltproc], [/usr/bin/xsltproc])
+  AC_PATH_PROG([XMLLINT], [xmllint], [])
+  if test -z "$XMLLINT"
+  then
+AC_MSG_ERROR("xmllint is required to build libvirt")
+  fi
+  AC_PATH_PROG([XSLTPROC], [xsltproc], [])
+  if test -z "$XSLTPROC"
+  then
+AC_MSG_ERROR("xsltproc is required to build libvirt")
+  fi
   AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
   AC_PROG_MKDIR_P
   AC_PROG_LN_S
-- 
2.13.3

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

[libvirt] [PATCH 02/12] docs: drop XHTML 1.0 validation of website

[Daniel P. Berrange]

The HTML pages are currently validated against an XHTML 1.0 DTD.
This makes it impossible to take advantage of features that are
introduced in HTML 5, because they'll fail validation.

There is intentionally no DTD defined for HTML 5, so there's no
alternative to XHTML 1.0 DTD that we could switch to. The only
options are to stick with XHTML 1.0 forever, or drop the DTD
validation, and we pick the latter.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 .travis.yml  |  1 -
 docs/Makefile.am | 18 +-
 libvirt.spec.in  |  1 -
 m4/virt-external-programs.m4 |  1 -
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 5a3e76510..f05ba8454 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,7 +38,6 @@ addons:
   - libapparmor-dev
   - dnsmasq-base
   - librbd-dev
-  - w3c-dtd-xhtml
 
 notifications:
   irc:
diff --git a/docs/Makefile.am b/docs/Makefile.am
index e32758f4a..f478d9505 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -260,14 +260,10 @@ MAINTAINERCLEANFILES += \
|| { rm $@ && exit 1; }; fi
 
 %.html: %.html.tmp
-   @if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \
- if $(XMLCATALOG) '$(XML_CATALOG_FILE)' \
-   "-//W3C//DTD XHTML 1.0 Strict//EN" > /dev/null ; then \
+   @if test -x $(XMLLINT) ; then \
  echo "Validating $@" ; \
- SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
- $(XMLLINT) --catalogs --nonet --format --valid $< > $(srcdir)/$@ \
- || { rm $(srcdir)/$@ && exit 1; }; \
- else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
+ $(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
+ || { rm $(srcdir)/$@ && exit 1; }; fi
 
 %.php.tmp: %.php.in site.xsl page.xsl
@if [ -x $(XSLTPROC) ] ; then \
@@ -290,12 +286,8 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl 
$(APIBUILD_STAMP)
  $(XSLTPROC) --nonet -o $(srcdir)/ \
  --stringparam builddir '$(abs_top_builddir)' \
  $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml ; fi && \
-   if test -x $(XMLLINT) && test -x $(XMLCATALOG) ; then \
- if $(XMLCATALOG) '$(XML_CATALOG_FILE)' "-//W3C//DTD XHTML 1.0 
Strict//EN" \
-   > /dev/null ; then \
- SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
- $(XMLLINT) --catalogs --nonet --valid --noout $(srcdir)/html/*.html ; 
\
- else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
+   if test -x $(XMLLINT) ; then \
+ $(XMLLINT) --nonet --noout $(srcdir)/html/*.html ; fi
 
 $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
$(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b074bd171..d1cff4caf 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -301,7 +301,6 @@ BuildRequires: systemd-units
 BuildRequires: xen-devel
 %endif
 BuildRequires: libxml2-devel
-BuildRequires: xhtml1-dtds
 BuildRequires: libxslt
 BuildRequires: readline-devel
 BuildRequires: ncurses-devel
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index f2f62f492..4a10c85ad 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -24,7 +24,6 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
 
   dnl Miscellaneous external programs.
   AC_PATH_PROG([XMLLINT], [xmllint], [/usr/bin/xmllint])
-  AC_PATH_PROG([XMLCATALOG], [xmlcatalog], [/usr/bin/xmlcatalog])
   AC_PATH_PROG([XSLTPROC], [xsltproc], [/usr/bin/xsltproc])
   AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
   AC_PROG_MKDIR_P
-- 
2.13.3

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

[libvirt] [PATCH 07/12] docs: use UTF-8 instead of HTML entities for decorated letters

[Daniel P. Berrange]

We have files which use HTML entities for decorating letters
with unlauts, accents, etc. Other files just use UTF-8
characters directly for this. Remove the HTML entities since
they have no benefit and use UTF-8 instead.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/apps.html.in   | 2 +-
 docs/csharp.html.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 760004715..e54ab5372 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -353,7 +353,7 @@
   
   http://honk.sigxcpu.org/projects/libvirt/#munin;>Munin
   
-The plugins provided by Guido Gnther allow to monitor various 
things
+The plugins provided by Guido Günther allow to monitor various things
 like network and block I/O with
 http://munin.projects.linpro.no/;>Munin.
   
diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index 722b02911..72746762a 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -115,7 +115,7 @@ git clone git://libvirt.org/libvirt-csharp.git
 
   The C# bindings are the work of Arnaud Champion
   mailto:arnaud.champion AT devatom.fr">arnaud.champion AT 
devatom.fr,
-  based upon the previous work of Jaromr ervenka.
+  based upon the previous work of Jaromír Červenka.
 
 
 Test Configuration
-- 
2.13.3

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

[libvirt] [PATCH 10/12] docs: remove bogus 'shape' attribute on links

[Daniel P. Berrange]

The 'shape' attribute on  is used together with a 'coords'
attribute to create hot-zones in image maps. We're not using
image maps so our inclusion of a 'shape' attribute is bogus.
Furthermore this is forbidden in HTML5.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/apps.html.in  |  4 ++--
 docs/format.html.in| 20 ++--
 docs/index.html.in | 20 ++--
 docs/news-2005.html.in |  4 ++--
 docs/news-2006.html.in |  4 ++--
 docs/news-2007.html.in |  4 ++--
 docs/news-2008.html.in |  4 ++--
 docs/news-2009.html.in |  4 ++--
 docs/news-2010.html.in |  4 ++--
 docs/news-2011.html.in |  4 ++--
 docs/news-2012.html.in |  4 ++--
 docs/news-2013.html.in |  4 ++--
 12 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/docs/apps.html.in b/docs/apps.html.in
index 2b91c4c03..1ced03c3d 100644
--- a/docs/apps.html.in
+++ b/docs/apps.html.in
@@ -365,10 +365,10 @@
 your Xen or QEMU/KVM guests, or to integrate with your existing Nagios
 installation.
   
-  http://www.pcp.io/man/man1/pmdalibvirt.1.html; 
shape="rect">PCP
+  http://www.pcp.io/man/man1/pmdalibvirt.1.html;>PCP
   
 The PCP libvirt PMDA (plugin) is part of the
-http://pcp.io/; shape="rect">PCP toolkit and provides
+http://pcp.io/;>PCP toolkit and provides
 hypervisor and guest information and complete set of guest performance
 metrics. It supports pCPU, vCPU, memory, block device, network 
interface,
 and performance event metrics for each virtual guest.
diff --git a/docs/format.html.in b/docs/format.html.in
index 11e0defb5..22b23e3fc 100644
--- a/docs/format.html.in
+++ b/docs/format.html.in
@@ -14,16 +14,16 @@
 
 
 
-  Domains
-  Networks
-  Network filtering
-  Storage
-  Storage 
encryption
-  Capabilities
-  Domain 
capabilities
-  Node devices
-  Secrets
-  Snapshots
+  Domains
+  Networks
+  Network filtering
+  Storage
+  Storage encryption
+  Capabilities
+  Domain capabilities
+  Node devices
+  Secrets
+  Snapshots
 
 
 Command line validation
diff --git a/docs/index.html.in b/docs/index.html.in
index 200f8eb10..8333cf6bb 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -59,16 +59,16 @@
 
 XML configuration
 Description of the XML schemas for
-  domains,
-  networks,
-  network filtering,
-  storage,
-  storage 
encryption,
-  capabilities,
-  domain capabilities,
-  node devices,
-  secrets,
-  snapshots
+  domains,
+  networks,
+  network filtering,
+  storage,
+  storage encryption,
+  capabilities,
+  domain capabilities,
+  node devices,
+  secrets,
+  snapshots
 http://wiki.libvirt.org;>Wiki
 Read further community contributed content
   
diff --git a/docs/news-2005.html.in b/docs/news-2005.html.in
index 6804cf2c9..44d8c4cc4 100644
--- a/docs/news-2005.html.in
+++ b/docs/news-2005.html.in
@@ -9,9 +9,9 @@
 Here is the list of official releases made during the year 2005.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log;>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2006.html.in b/docs/news-2006.html.in
index 558f55a98..6556d4fed 100644
--- a/docs/news-2006.html.in
+++ b/docs/news-2006.html.in
@@ -10,9 +10,9 @@
 A similar list for 2005 is also available.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log;>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2007.html.in b/docs/news-2007.html.in
index 46d8457cb..c12449abf 100644
--- a/docs/news-2007.html.in
+++ b/docs/news-2007.html.in
@@ -10,9 +10,9 @@
 A similar list for 2006 is also available.
 
 It is also possible to just use
-the GIT version or snapshot,
+the GIT version or snapshot,
 contact the mailing list and check
-the http://libvirt.org/git/?p=libvirt.git;a=log; shape="rect">GIT 
log
+the http://libvirt.org/git/?p=libvirt.git;a=log;>GIT log
 to gauge progress.
 
 
diff --git a/docs/news-2008.html.in b/docs/news-2008.html.in
index 8081b82f1..aefbf3873 100644
--- a/docs/news-2008.html.in
+++ b/docs/news-2008.html.in
@@ -10,9 +10,9 @@
 A similar list for 2007 is also available.
 
 It is also possible to just use
-the

[libvirt] [PATCH 05/12] docs: remove use of in docs

[Daniel P. Berrange]

Some docs pages were using  to add arbitrary whitespace
in the page. This is something that should be done by CSS if needed,
but it is not needed here, so delete it.

There was also use of  which adds no value at all
when we have CSS to prettify tables.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 docs/csharp.html.in  | 612 +++
 docs/virshcmdref.html.in |   6 -
 2 files changed, 300 insertions(+), 318 deletions(-)

diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index e1c0fefba..722b02911 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -19,8 +19,6 @@
   to libvirt.
 
 
-
-
 Requirements
 
 
@@ -31,8 +29,6 @@
   compiling libvirt for windows.
 
 
-
-
 

[libvirt] [PATCH 00/12] Cleanup website generation & add favicons

[Daniel P. Berrange]

This started as an attempt to add modern favicon support to
the website. This requires use of HTML5 only syntax, which
lead to the massive cleanup to stop using XHTML 1.0, which
forms all of this series except the last patch

Daniel P. Berrange (12):
  docs: switch to using 'id' attribute instead of 'name' for links
  docs: drop XHTML 1.0 validation of website
  docs: make xmllint & xsltproc compulsory
  docs: fix typo s///
  docs: remove use of  in docs
  docs: remove use of  entity
  docs: use UTF-8 instead of HTML entities for decorated letters
  docs: switch to using HTML5 doctype declaration
  docs: generate pretty indented HTML for API docs
  docs: remove bogus 'shape' attribute on links
  docs: explicitly declare pages as being UTF-8 format
  docs: add full set of "favicon" files to support modern clients

 .travis.yml  |   1 -
 docs/404.html.in |   2 +-
 docs/Makefile.am |  52 +--
 docs/acl.html.in |   8 +-
 docs/aclpolkit.html.in   |  34 +-
 docs/android-chrome-192x192.png  | Bin 0 -> 13057 bytes
 docs/android-chrome-256x256.png  | Bin 0 -> 16597 bytes
 docs/api.html.in |  10 +-
 docs/api_extension.html.in   |  14 +-
 docs/apple-touch-icon.png| Bin 0 -> 10489 bytes
 docs/apps.html.in|  38 +--
 docs/architecture.html.in|   8 +-
 docs/auditlog.html.in|  40 +--
 docs/auth.html.in|  16 +-
 docs/bindings.html.in|   2 +-
 docs/browserconfig.xml   |   9 +
 docs/bugs.html.in|  12 +-
 docs/cgroups.html.in |  26 +-
 docs/compiling.html.in   |   8 +-
 docs/contact.html.in |   8 +-
 docs/contribute.html.in  |  12 +-
 docs/csharp.html.in  | 634 +--
 docs/devguide.html.in|   2 +-
 docs/docs.html.in|   2 +-
 docs/downloads.html.in   |  14 +-
 docs/drivers.html.in |   6 +-
 docs/drvbhyve.html.in|  22 +-
 docs/drvesx.html.in  |  46 +--
 docs/drvhyperv.html.in   |  12 +-
 docs/drvlxc.html.in  |  54 +--
 docs/drvnodedev.html.in  |  10 +-
 docs/drvopenvz.html.in   |  14 +-
 docs/drvphyp.html.in |   8 +-
 docs/drvqemu.html.in |  34 +-
 docs/drvremote.html.in   |   2 +-
 docs/drvtest.html.in |   2 +-
 docs/drvuml.html.in  |   4 +-
 docs/drvvbox.html.in |   6 +-
 docs/drvvirtuozzo.html.in|   8 +-
 docs/drvvmware.html.in   |   6 +-
 docs/drvxen.html.in  |  16 +-
 docs/errors.html.in  |   2 +-
 docs/favicon-16x16.png   | Bin 0 -> 872 bytes
 docs/favicon-32x32.png   | Bin 0 -> 1793 bytes
 docs/favicon.ico | Bin 0 -> 15086 bytes
 docs/firewall.html.in|   8 +-
 docs/format.html.in  |  22 +-
 docs/formatcaps.html.in  |  10 +-
 docs/formatdomain.html.in| 228 ++---
 docs/formatdomaincaps.html.in|  26 +-
 docs/formatnetwork.html.in   |  36 +-
 docs/formatnode.html.in  |   6 +-
 docs/formatnwfilter.html.in  |  70 ++--
 docs/formatsecret.html.in|  12 +-
 docs/formatsnapshot.html.in  |   6 +-
 docs/formatstorage.html.in   |  30 +-
 docs/formatstorageencryption.html.in |  12 +-
 docs/genaclperms.pl  |   2 +-
 docs/goals.html.in   |   2 +-
 docs/governance.html.in  |  14 +-
 docs/hacking.html.in |  42 +--
 docs/hooks.html.in   |  34 +-
 docs/hvsupport.pl|   2 +-
 docs/index.html.in   |  22 +-
 docs/internals.html.in   |   2 +-
 docs/internals/command.html.in   |  34 +-
 docs/internals/eventloop.html.in |   8 +-
 docs/internals/locking.html.in   |  18 +-
 docs/internals/oomtesting.html.in|  10 +-
 docs/internals/rpc.html.in   |  44 +--
 docs/java.html.in|   2 +-
 docs/locking-lockd.html.in   |  10 +-
 docs/locking-sanlock.html.in |  12 +-
 docs/locking.html.in |   4 +-
 docs/logging.html.in |  16 +-
 docs/manifest.json   |  18 +
 docs/migration.html.in   |  38 +--
 docs/mstile-150x150.png  | Bin 0 -> 7579 bytes
 docs/newapi.xsl  |  19 +-
 docs/news-2005.html.in   |   6 +-
 docs/news-2006.html.in   |   6 +-
 docs/news-2007.html.in   |   6 +-
 docs/news-2008.html.in   |   6 +-
 docs/news-2009.html.in   |   6 +-
 docs/news-2010.html.in

Re: [libvirt] [PATCH] docs: Add build timestamps to generated html/php pages

[Daniel P. Berrange]

On Wed, Jul 26, 2017 at 05:13:08PM +0200, Martin Kletzander wrote:
> In order not to make the build even less reproducible, honour
> SOURCE_DATE_EPOCH environment variable as specified:
> 
>   https://reproducible-builds.org/specs/source-date-epoch/
> 
> Signed-off-by: Martin Kletzander <mklet...@redhat.com>
> ---
>  docs/Makefile.am | 13 +
>  docs/newapi.xsl  |  2 ++
>  docs/page.xsl|  4 
>  docs/site.xsl|  1 +
>  docs/subsite.xsl |  1 +
>  5 files changed, 17 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH glib] Don't set LC_ALL=C during build as that breaks python apps

[Daniel P. Berrange]

On Tue, Jul 25, 2017 at 06:16:49PM +0200, Philipp Hahn wrote:
> Hello,
> 
> Am 25.07.2017 um 14:07 schrieb Daniel P. Berrange:
> > Setting LC_ALL=C breaks python apps doing I/O on UTF-8 source
> > files. In particular this broke glib-mkenums
> > 
> >   GEN  libvirt-gconfig-enum-types.h
> > Traceback (most recent call last):
> >   File "/usr/bin/glib-mkenums", line 669, in 
> > process_file(fname)
> >   File "/usr/bin/glib-mkenums", line 406, in process_file
> > line = curfile.readline()
> >   File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
> > return codecs.ascii_decode(input, self.errors)[0]
> > UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 849: 
> > ordinal not in range(128)
> 
> What about using "C.UTF-8" instead, which us the same as "C" but with
> "UTF-8" encoding?
> Maybe ancient RedHat still doesn't know about it ...

It isn't portable.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH glib] Don't set LC_ALL=C during build as that breaks python apps

[Daniel P. Berrange]

Setting LC_ALL=C breaks python apps doing I/O on UTF-8 source
files. In particular this broke glib-mkenums

  GEN  libvirt-gconfig-enum-types.h
Traceback (most recent call last):
  File "/usr/bin/glib-mkenums", line 669, in 
process_file(fname)
  File "/usr/bin/glib-mkenums", line 406, in process_file
line = curfile.readline()
  File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 849: 
ordinal not in range(128)

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed to fix rawhide build

 maint.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/maint.mk b/maint.mk
index 405c6d0..ef72b4f 100644
--- a/maint.mk
+++ b/maint.mk
@@ -117,8 +117,8 @@ news-check-lines-spec ?= 1,10
 news-check-regexp ?= '^\*.* $(VERSION_REGEXP) \($(today)\)'
 
 # Prevent programs like 'sort' from considering distinct strings to be equal.
-# Doing it here saves us from having to set LC_ALL elsewhere in this file.
-export LC_ALL = C
+# Doing it here saves us from having to set LC_COLLATE elsewhere in this file.
+export LC_COLLATE = C
 
 ## --- ##
 ## Sanity checks.  ##
-- 
2.13.3

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

Re: [libvirt] [PATCH] Revert "virthread: Introduce virRWLockInitPreferWriter"

[Daniel P. Berrange]

On Tue, Jul 25, 2017 at 10:43:50AM +0200, Michal Privoznik wrote:
> This reverts commit 328bd24443d2a345a5832ee48ebba0208f8036ea.
> 
> As it turns out, this is not portable and very Linux & glibc
> specific. Worse, this may lead to not starving writers on Linux
> but everywhere else. Revert this and if the starvation occurs
> resolve it.
> 
> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 1/3] virthread: Introduce virRWLockInitPreferWriter

[Daniel P. Berrange]

On Mon, Jul 24, 2017 at 01:12:59PM -0400, John Ferlan wrote:
> 
> 
> On 07/19/2017 10:31 AM, Michal Privoznik wrote:
> > We already have virRWLockInit. But this uses pthread defaults
> > which prefer reader to initialize the RW lock. This may lead to
> > writer starvation. Therefore we need to have the counterpart that
> > prefers writers. Now, according to the
> > pthread_rwlockattr_setkind_np() man page setting
> > PTHREAD_RWLOCK_PREFER_WRITER_NP attribute is no-op. Therefore we
> > need to use PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
> > attribute. So much for good enum value names.
> > 
> > Signed-off-by: Michal Privoznik 
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/virthread.c | 35 +++
> >  src/util/virthread.h |  1 +
> >  3 files changed, 37 insertions(+)
> > 
> 
> This has broken the CI build, freebsd is not happy:
> 
> ../../src/util/virthread.c:133:42: error: use of undeclared identifier
> 'PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP'
> pthread_rwlockattr_setkind_np(,
> PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
>  ^
> 1 error generated.

It is not just FreeBSD, it also breaks OS-X and Win32.

The suffix '_np' / '_NP' is shorthand for nNon portable" - these
are glibc inventions. IMHO we should not really use this in our
code as if we're going to make assumptions that writers are not
starved as a result, because writers will be starved on any
non-Linux platform.

IOW, I think we need to just revert this.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] libvirt-domain.h: Fix enum description placement

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 04:33:06PM +0200, Jiri Denemark wrote:
> On Fri, Jul 21, 2017 at 14:42:05 +0100, Daniel P. Berrange wrote:
> > On Fri, Jul 21, 2017 at 03:00:20PM +0200, Martin Kletzander wrote:
> 
> > > struct meh {
> > >   /*# This is comment for the following member foo */
> > >   unsigned int foo;
> > >   int bar; /*< This is for member bar that's on the same line */
> > > }
> > > 
> > > and so on.  If that doesn't help either and it never worked,
> > > then... it's a pity :-/
> > 
> > That is ambiguous - without seeing whitespace, the parser cannot
> > distinguish between these two scenarios:
> > 
> >  struct meh {
> >unsigned int foo; /*# This is comment for the following member foo */
> >int bar;
> >  }
> 
> Martin suggested < to be used instead of # for this type of comments to
> remove the ambiguity.
> 
> >  struct meh {
> >unsigned int foo;
> >
> > /*# This is comment for the following member foo */
> >int bar;
> >  }
> 
> However, I think we could just simply force using only comments above
> each member in our public header files and fix the generator to properly
> work with them. This should be a lot easier than fixing it to handle
> both options.

I'd probably go for doing that. In many cases where we put the comment
at the end of the line, we're forced to split it across many lines
anyway, lest the line get too long. Putting commments before the
item means we won't hit the line length so easily, so more comments
will fit on one line.

Oh and this issue affects struct field members too IIUC

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] libvirt-domain.h: Fix enum description placement

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 03:00:20PM +0200, Martin Kletzander wrote:
> On Fri, Jul 21, 2017 at 11:58:45AM +0100, Daniel P. Berrange wrote:
> > On Fri, Jul 21, 2017 at 12:52:06PM +0200, Michal Privoznik wrote:
> > > There are only two acceptable places for describing enum values.
> > > It's either:
> > > 
> > > typedef enum {
> > > /* Some long description. Therefore it's placed before
> > >  * the value. */
> > > VIR_ENUM_A_VAL = 1,
> > > } virEnumA;
> > > 
> > > or:
> > > 
> > > typedef enum {
> > > VIR_ENUM_B_VAL = 1, /* Some short description */
> > > } virEnumB;
> > > 
> > > However, during review of a patch sent upstream I realized that
> > > is not always the case. I went through all the public header
> > > files and identified all the offenders. Luckily there were just
> > > two of them.
> > > 
> > > Yes, this makes our HTML generated documentation broken, but
> > > that's bug of the generator. Our header files shouldn't be forced
> > > to use something we don't want to.
> > 
> > FWIW, the problem is in the parseEnumBLock() method in apibuild.py
> > 
> > Once it has completed parsing an enum item, it delays adding that
> > enum to the list until it sees the next item, so that it can capture
> > the trailing comment.
> > 
> > The only way we can distinguish between a comment that comes before
> > the enum vs a comment after the enum, on the same line, is by detecting
> > whitespace (newline) before the trailing comment. Unfortunately I don't
> > thing this is exposed by the lexer right, so its not entirely easy
> > to solve.
> > 
> 
> I was under the impression that this worked, we only broke it by some
> recent commit.  I looked at the code and got pretty confused by it, but
> shouldn't it be pretty easy (from a big picture view)?  You read until
> you have both comment and a member of the struct.
> 
> If it's really harder than I think, then we can start using some helper
> characters for the comments (at least for now) so that we can properly
> identify them, e.g.:
> 
> struct meh {
>   /*# This is comment for the following member foo */
>   unsigned int foo;
>   int bar; /*< This is for member bar that's on the same line */
> }
> 
> and so on.  If that doesn't help either and it never worked,
> then... it's a pity :-/

That is ambiguous - without seeing whitespace, the parser cannot
distinguish between these two scenarios:

 struct meh {
   unsigned int foo; /*# This is comment for the following member foo */
   int bar;
 }

 struct meh {
   unsigned int foo;
   
/*# This is comment for the following member foo */
   int bar;
 }




Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 01:03:13PM +0200, Michal Privoznik wrote:
> On 07/21/2017 01:01 PM, Daniel P. Berrange wrote:
> > On Fri, Jul 21, 2017 at 12:58:55PM +0200, Michal Privoznik wrote:
> >> 
> >> Yeah. Our generator is not that great. I wish that we'd switch to
> >> something already out there so that we don't have to maintain our own
> >> generator.
> > 
> > There's a good few options for docs generators. Unfortunately our code
> > is also used for generating the XML API description, that's in turn
> > used to generate some language bindings, and I don't know of any tools
> > that could replace that part. So it looks like we're stuck as long as
> > we want to support auto-generated bindings from the XML description
> 
> Well we can use proper doc generator for generating doc and then our own
> generator for generating the XML API description. BTW: what good options
> do you have in mind?

Doxygen, Sphinx, GTK-DOC, though I would not be in favour of Doxygen as
IMHO the docs it generates have awful navigation. QEMU is switching to
use Sphinx IIRC.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 12:58:55PM +0200, Michal Privoznik wrote:
> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
> > On Fri, 21 Jul 2017 10:12:38 +0200
> > Michal Privoznik  wrote:
> > 
> >> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> >>> The documentation string has to follow the definition of a constant in
> >>> the enum. Otherwise, the HTML documentation will be generated
> >>> incorrectly.
> >>>
> >>> Signed-off-by: Tomáš Golembiovský 
> >>> ---
> >>>  include/libvirt/libvirt-domain.h | 62 
> >>> 
> >>>  1 file changed, 31 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/include/libvirt/libvirt-domain.h 
> >>> b/include/libvirt/libvirt-domain.h
> >>> index 45f939a8c..2f3162d0f 100644
> >>> --- a/include/libvirt/libvirt-domain.h
> >>> +++ b/include/libvirt/libvirt-domain.h
> >>> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> >>> *virDomainInterfaceStatsPtr;
> >>>   * Memory Statistics Tags:
> >>>   */
> >>>  typedef enum {
> >>> -/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> >>> -/* The total amount of memory written out to swap space (in kB). */
> >>> +/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> >>> +/* The total amount of memory written out to swap space (in kB). */  
> >>
> >> While this fixes generated HTML, it messes up the header file. So if
> >> somebody is looking directly into header file they might get confused.
> >> The problem is in our doc generator.
> > 
> > I agree that it's not ideal solution. But since it's already used in the
> > header, e.g. in virDomainBlockJobType and
> > virDomainEventShutdownDetailType, 
> 
> This one actually looks okay. Did you perhaps mean
> virConnectDomainEventDiskChangeReason?
> 
> > I assumed it's acceptable. And the
> > overall readability is not that bad because the constant+doc pairs are
> > separated with newline from one another.
> 
> Nope. It's not. I've sent a patch that fixes those two places (I've went
> through all of our public headers and found just those two though):
> 
> https://www.redhat.com/archives/libvir-list/2017-July/msg00871.html
> 
> > 
> > 
> > 
> >> I recall this being discussed
> >> somewhere recently (probably on the list?). The proper fix IMO is to fix
> >> the generator so that it accepts both:
> > 
> > That would be the best solution, but I'm too scared to look at the
> > generator. That would be a job for somebody else.
> 
> Yeah. Our generator is not that great. I wish that we'd switch to
> something already out there so that we don't have to maintain our own
> generator.

There's a good few options for docs generators. Unfortunately our code
is also used for generating the XML API description, that's in turn
used to generate some language bindings, and I don't know of any tools
that could replace that part. So it looks like we're stuck as long as
we want to support auto-generated bindings from the XML description


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] libvirt-domain.h: Fix enum description placement

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 12:52:06PM +0200, Michal Privoznik wrote:
> There are only two acceptable places for describing enum values.
> It's either:
> 
> typedef enum {
> /* Some long description. Therefore it's placed before
>  * the value. */
> VIR_ENUM_A_VAL = 1,
> } virEnumA;
> 
> or:
> 
> typedef enum {
> VIR_ENUM_B_VAL = 1, /* Some short description */
> } virEnumB;
> 
> However, during review of a patch sent upstream I realized that
> is not always the case. I went through all the public header
> files and identified all the offenders. Luckily there were just
> two of them.
> 
> Yes, this makes our HTML generated documentation broken, but
> that's bug of the generator. Our header files shouldn't be forced
> to use something we don't want to.

FWIW, the problem is in the parseEnumBLock() method in apibuild.py

Once it has completed parsing an enum item, it delays adding that
enum to the list until it sees the next item, so that it can capture
the trailing comment.

The only way we can distinguish between a comment that comes before
the enum vs a comment after the enum, on the same line, is by detecting
whitespace (newline) before the trailing comment. Unfortunately I don't
thing this is exposed by the lexer right, so its not entirely easy
to solve.


> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
> ---
>  include/libvirt/libvirt-domain.h | 17 +++++
>  1 file changed, 9 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 12:25:02PM +0200, Tomáš Golembiovský wrote:
> On Fri, 21 Jul 2017 10:12:38 +0200
> Michal Privoznik  wrote:
> 
> > On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > > The documentation string has to follow the definition of a constant in
> > > the enum. Otherwise, the HTML documentation will be generated
> > > incorrectly.
> > > 
> > > Signed-off-by: Tomáš Golembiovský 
> > > ---
> > >  include/libvirt/libvirt-domain.h | 62 
> > > 
> > >  1 file changed, 31 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/include/libvirt/libvirt-domain.h 
> > > b/include/libvirt/libvirt-domain.h
> > > index 45f939a8c..2f3162d0f 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > > *virDomainInterfaceStatsPtr;
> > >   * Memory Statistics Tags:
> > >   */
> > >  typedef enum {
> > > -/* The total amount of data read from swap space (in kB). */
> > >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > > -/* The total amount of memory written out to swap space (in kB). */
> > > +/* The total amount of data read from swap space (in kB). */
> > >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > > +/* The total amount of memory written out to swap space (in kB). */  
> > 
> > While this fixes generated HTML, it messes up the header file. So if
> > somebody is looking directly into header file they might get confused.
> > The problem is in our doc generator.
> 
> I agree that it's not ideal solution. But since it's already used in the
> header, e.g. in virDomainBlockJobType and
> virDomainEventShutdownDetailType, I assumed it's acceptable. And the
> overall readability is not that bad because the constant+doc pairs are
> separated with newline from one another.

I'm afraid, those examples are bad too and should never have been committed
as they are, so not a good guide to follow.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

[Daniel P. Berrange]

On Fri, Jul 21, 2017 at 10:12:38AM +0200, Michal Privoznik wrote:
> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > The documentation string has to follow the definition of a constant in
> > the enum. Otherwise, the HTML documentation will be generated
> > incorrectly.
> > 
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  include/libvirt/libvirt-domain.h | 62 
> > 
> >  1 file changed, 31 insertions(+), 31 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 45f939a8c..2f3162d0f 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > *virDomainInterfaceStatsPtr;
> >   * Memory Statistics Tags:
> >   */
> >  typedef enum {
> > -/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > -/* The total amount of memory written out to swap space (in kB). */
> > +/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > +/* The total amount of memory written out to swap space (in kB). */
> 
> While this fixes generated HTML, it messes up the header file. So if
> somebody is looking directly into header file they might get confused.

Yeah, absolutely NACK to this approach. Putting comments after the
constant name is awful for people reading the code.

> The problem is in our doc generator. I recall this being discussed
> somewhere recently (probably on the list?). The proper fix IMO is to fix
> the generator so that it accepts both:
> 
> enum {
>   /* Some very long description - therefore it's before the value. */
>   VIR_ENUM_A_VAL = 0,
> } virEnumA;
> 
> enum {
>   VIR_ENUM_B_VAL = 0, /* Short description */
> } virEnumB;
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v1] numa: compute and set matching vcpus for numa domains

[Daniel P. Berrange]

On Thu, Jul 20, 2017 at 03:03:25PM +0200, Wim ten Have wrote:
> On Thu, 20 Jul 2017 11:10:31 +0100
> "Daniel P. Berrange" <berra...@redhat.com> wrote:
> 
> > On Thu, Jul 20, 2017 at 11:29:26AM +0200, Wim Ten Have wrote:
> > > From: Wim ten Have <wim.ten.h...@oracle.com>
> > > 
> > > The QEMU driver can erroneously allocate more vpus to a domain
> > > than there are cpus in the domain if the  element is used
> > > to describe  element topology. Fix this by calculating
> > > the number of cpus described in the  element of a 
> > > element and comparing it to the number of vcpus. If the number
> > > of vcpus is greater than the number of cpus, cap the number of
> > > vcpus to the number of cpus.
> > > 
> > > This patch also adds a small libvirt documentation update under
> > > the "CPU Allocation" section.
> > > 
> > > Signed-off-by: Wim ten Have <wim.ten.h...@oracle.com>
> > > ---
> > >  docs/formatdomain.html.in |  9 -
> > >  src/conf/domain_conf.c| 14 +++---
> > >  2 files changed, 19 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > > index 07208ee..3c85307 100644
> > > --- a/docs/formatdomain.html.in
> > > +++ b/docs/formatdomain.html.in
> > > @@ -501,7 +501,14 @@
> > >vcpu
> > >The content of this element defines the maximum number of 
> > > virtual
> > >  CPUs allocated for the guest OS, which must be between 1 and
> > > -the maximum supported by the hypervisor.
> > > +the maximum supported by the hypervisor. If a numa
> > > +element is contained within the cpu element, the
> > > +number of virtual CPUs to be allocated is compared with the sum
> > > +of the cpus attributes across the cell
> > > +elements within the numa element. If the number of
> > > +virtual CPUs is greater than the sum of the cpus
> > > +attributes, the number of virtual CPUs is capped at sum of the
> > > +cpus attributes.
> > >  
> > >   cpuset
> > >   
> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > index 958a5b7..73d7f4f 100644
> > > --- a/src/conf/domain_conf.c
> > > +++ b/src/conf/domain_conf.c
> > > @@ -3844,12 +3844,20 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
> > >  unsigned long long numaMemory = 0;
> > >  unsigned long long hotplugMemory = 0;
> > >  
> > > -/* Attempt to infer the initial memory size from the sum NUMA memory 
> > > sizes
> > > - * in case ABI updates are allowed or the  element wasn't 
> > > specified */
> > > +/* Attempt to infer the initial memory size from the sum NUMA memory
> > > + * sizes in case ABI updates are allowed or the  element
> > > + * wasn't specified.  Also cap the vcpu count to the sum of NUMA cpus
> > > + * allocated for all nodes. */
> > >  if (def->mem.total_memory == 0 ||
> > >  parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE ||
> > > -parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION)
> > > +parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION) {
> > > +unsigned int numaVcpus = 0;
> > > +
> > > +if ((numaVcpus = virDomainNumaGetCPUCountTotal(def->numa)))
> > > +virDomainDefSetVcpus(def, numaVcpus);
> > > +
> > >  numaMemory = virDomainNumaGetMemorySize(def->numa);
> > > +}  
> > 
> > AFAICT, this scenario is simply a user configuration mistake, and so we
> > should report an error message to them to fix it.
> 
>   Not to push but I think this is the correct way ... O:-).
> 
>   Ok, perhaps the documentation note/commit message should be slightly
>   rewritten aswell as the altered comment on specific code section.
> 
>   The change is _NOT_ changing final guest provided 'maxvcpus' but
>   'vcpus' instead.  This means that it adds the "current='#count'" under
>   the vcpu element if such is less then vcpu 'maxvcpus' provided count.
>   If equal there is no issue and if larger there is indeed a correct
>   error message (exceptional condition).
> 
>   Imagine simpel domain description _WITHOUT_  and below 
>   element description;
> 
> 16
> 
>   This nicely creates a guest with 4 d

Re: [libvirt] [PATCH v1] numa: compute and set matching vcpus for numa domains

[Daniel P. Berrange]

On Thu, Jul 20, 2017 at 11:29:26AM +0200, Wim Ten Have wrote:
> From: Wim ten Have 
> 
> The QEMU driver can erroneously allocate more vpus to a domain
> than there are cpus in the domain if the  element is used
> to describe  element topology. Fix this by calculating
> the number of cpus described in the  element of a 
> element and comparing it to the number of vcpus. If the number
> of vcpus is greater than the number of cpus, cap the number of
> vcpus to the number of cpus.
> 
> This patch also adds a small libvirt documentation update under
> the "CPU Allocation" section.
> 
> Signed-off-by: Wim ten Have 
> ---
>  docs/formatdomain.html.in |  9 -
>  src/conf/domain_conf.c| 14 +++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 07208ee..3c85307 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -501,7 +501,14 @@
>vcpu
>The content of this element defines the maximum number of virtual
>  CPUs allocated for the guest OS, which must be between 1 and
> -the maximum supported by the hypervisor.
> +the maximum supported by the hypervisor. If a numa
> +element is contained within the cpu element, the
> +number of virtual CPUs to be allocated is compared with the sum
> +of the cpus attributes across the cell
> +elements within the numa element. If the number of
> +virtual CPUs is greater than the sum of the cpus
> +attributes, the number of virtual CPUs is capped at sum of the
> +cpus attributes.
>  
>   cpuset
>   
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 958a5b7..73d7f4f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3844,12 +3844,20 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
>  unsigned long long numaMemory = 0;
>  unsigned long long hotplugMemory = 0;
>  
> -/* Attempt to infer the initial memory size from the sum NUMA memory 
> sizes
> - * in case ABI updates are allowed or the  element wasn't 
> specified */
> +/* Attempt to infer the initial memory size from the sum NUMA memory
> + * sizes in case ABI updates are allowed or the  element
> + * wasn't specified.  Also cap the vcpu count to the sum of NUMA cpus
> + * allocated for all nodes. */
>  if (def->mem.total_memory == 0 ||
>  parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE ||
> -parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION)
> +parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION) {
> +unsigned int numaVcpus = 0;
> +
> +if ((numaVcpus = virDomainNumaGetCPUCountTotal(def->numa)))
> +virDomainDefSetVcpus(def, numaVcpus);
> +
>  numaMemory = virDomainNumaGetMemorySize(def->numa);
> +}

AFAICT, this scenario is simply a user configuration mistake, and so we
should report an error message to them to fix it.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH go-xml] Add support for NAT in network forward

[Daniel P. Berrange]

On Wed, Jul 12, 2017 at 01:26:03PM +0200, Thomas Hipp wrote:
> Add support for NAT in network forward, and add test code.
> 
> Signed-off-by: Thomas Hipp 
> ---
>  network.go  | 20 ++--
>  network_test.go | 31 +--
>  2 files changed, 43 insertions(+), 8 deletions(-)

ACK & pushed


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH go-xml] Add support for DNS in network

[Daniel P. Berrange]

On Wed, Jul 12, 2017 at 03:28:13PM +0200, Thomas Hipp wrote:
> Add support for DNS in network, and add test code.
> 
> Signed-off-by: Thomas Hipp 
> ---
>  network.go  | 39 ++
>  network_test.go | 65 
> -
>  2 files changed, 99 insertions(+), 5 deletions(-)

ACK & pushed


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH RESEND] qemu: shared disks with cache=directsync should be safe for migration

[Daniel P. Berrange]

On Sat, Jul 15, 2017 at 11:01:25PM +0800, Peng Hao wrote:
> From: Hao Peng <peng.h...@zte.com.cn>
> 
> At present shared disks can be migrated with either readonly or cache=none. 
> But
> cache=directsync should be safe for migration, because both cache=directsync 
> and cache=none
> don't use the host page cache, and cache=direct write through qemu block 
> layer cache.
> 
> Signed-off-by: Peng Hao <peng.h...@zte.com.cn>
> Reviewed-by: Wang Yechao <wang.yechao...@zte.com.cn>
> ---
>  src/qemu/qemu_migration.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

and pushed to git master

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH jenkins-ci] Put all RPM build files in GIT checkout build dir

[Daniel P. Berrange]

The CI system slaves keep running out of space due to /home/jenkins/rpmbuild
filling up disk.  If we tell rpmbuild to put its files in `pwd`/rpmbuild
instead, that'll end up in the GIT checkout of the module in question. Jenkins
does a 'git clean' after checkout, so this conveniently blows away the RPM
files from previous builds preventing disk usage growing without bound.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---

Pushed to fix the CI system

 jobs/autotools.yaml| 2 +-
 jobs/perl-makemaker.yaml   | 2 +-
 jobs/perl-modulebuild.yaml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index a2241b2..3e4fdc2 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -171,7 +171,7 @@
   sed -i -e 's/BuildRequires: pkgconfig(libvirt.*).*//' {name}.spec
   rm -f *.tar.{archive_format}
   $MAKE -j{smp} dist
-  rpmbuild -ta {name}-*.tar.{archive_format}
+  rpmbuild --define "_topdir `pwd`/rpmbuild" -ta 
{name}-*.tar.{archive_format}
 publishers:
   - email:
   recipients: '{obj:spam}'
diff --git a/jobs/perl-makemaker.yaml b/jobs/perl-makemaker.yaml
index b13d7cc..2df5882 100644
--- a/jobs/perl-makemaker.yaml
+++ b/jobs/perl-makemaker.yaml
@@ -122,7 +122,7 @@
   sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec
   rm -f *.tar.{archive_format}
   make -j{smp} dist
-  rpmbuild -ta *.tar.{archive_format}
+  rpmbuild --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format}
 publishers:
   - email:
   recipients: '{obj:spam}'
diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml
index ba4d606..cb81de1 100644
--- a/jobs/perl-modulebuild.yaml
+++ b/jobs/perl-modulebuild.yaml
@@ -122,7 +122,7 @@
   sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec
   rm -f *.tar.{archive_format}
   perl Build dist
-  rpmbuild -ta *.tar.{archive_format}
+  rpmbuild --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format}
 publishers:
   - email:
   recipients: '{obj:spam}'
-- 
2.13.0

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