[libvirt] [PATCH] qemu: Fix NBD migration with default listenAddress

[Jiri Denemark]

My commit 674afcb09e3d33500cfbbcf870ebf92cb99ecfa3 moved computing the
default listen address from qemuMigrationPrepareAny to
qemuMigrationPrepareIncoming. However, I didn't notice listenAddress was
later passed to qemuMigrationStartNBDServer. Thus, it would be called
with the original value of listenAddress (NULL).

Let's add the updated listen address to qemuProcessIncomingDef and use
it when starting NBD servers.

Reported-by: Michael Chapman 
Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 5 +++--
 src/qemu/qemu_process.c   | 7 ++-
 src/qemu/qemu_process.h   | 2 ++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6be11b4..cb8c75d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3365,7 +3365,8 @@ qemuMigrationPrepareIncoming(virDomainObjPtr vm,
 goto cleanup;
 }
 
-inc = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, fd, NULL);
+inc = qemuProcessIncomingDefNew(priv->qemuCaps, listenAddress,
+migrateFrom, fd, NULL);
 
  cleanup:
 VIR_FREE(migrateFrom);
@@ -3586,7 +3587,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 if (mig->nbd &&
 flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) &&
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) {
-if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
+if (qemuMigrationStartNBDServer(driver, vm, incoming->address,
 nmigrate_disks, migrate_disks) < 0) {
 goto stopjob;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f274068..819ad05 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4124,6 +4124,7 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc)
 if (!inc)
 return;
 
+VIR_FREE(inc->address);
 VIR_FREE(inc->launchURI);
 VIR_FREE(inc->deferredURI);
 VIR_FREE(inc);
@@ -4137,6 +4138,7 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc)
  */
 qemuProcessIncomingDefPtr
 qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps,
+  const char *listenAddress,
   const char *migrateFrom,
   int fd,
   const char *path)
@@ -4149,6 +4151,9 @@ qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps,
 if (VIR_ALLOC(inc) < 0)
 return NULL;
 
+if (VIR_STRDUP(inc->address, listenAddress) < 0)
+goto error;
+
 inc->launchURI = qemuMigrationIncomingURI(migrateFrom, fd);
 if (!inc->launchURI)
 goto error;
@@ -5137,7 +5142,7 @@ qemuProcessStart(virConnectPtr conn,
 goto cleanup;
 
 if (migrateFrom) {
-incoming = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom,
+incoming = qemuProcessIncomingDefNew(priv->qemuCaps, NULL, migrateFrom,
  migrateFd, migratePath);
 if (!incoming)
 goto stop;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 85e3a06..c674111 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -47,6 +47,7 @@ int qemuProcessAssignPCIAddresses(virDomainDefPtr def);
 typedef struct _qemuProcessIncomingDef qemuProcessIncomingDef;
 typedef qemuProcessIncomingDef *qemuProcessIncomingDefPtr;
 struct _qemuProcessIncomingDef {
+char *address; /* address where QEMU is supposed to listen */
 char *launchURI; /* used as a parameter for -incoming command line option 
*/
 char *deferredURI; /* used when calling migrate-incoming QMP command */
 int fd; /* for fd:N URI */
@@ -54,6 +55,7 @@ struct _qemuProcessIncomingDef {
 };
 
 qemuProcessIncomingDefPtr qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps,
+const char *listenAddress,
 const char *migrateFrom,
 int fd,
 const char *path);
-- 
2.7.0

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

Re: [libvirt] [PATCH 00/21] Support NBD for tunnelled migration

[Jiri Denemark]

On Wed, Nov 18, 2015 at 20:12:58 +0200, Pavel Boldin wrote:
> The provided patchset implements NBD disk migration over a tunnelled
> connection provided by libvirt.
> 
> The migration source instructs QEMU to NBD mirror drives into the provided
> UNIX socket. These connections and all the data are then tunnelled to the
> destination using newly introduced RPC call. The migration destination
> implements a driver method that connects the tunnelled stream to the QEMU's
> NBD destination.

To be honest, I'm still in doubts this is all worth the effort. This
code will likely be unused once both QEMU and libvirt gain proper TLS
support for migration. The current tunneled migration is known to be
slow due to the big overhead and even with the presence of a better
alternative we'd still have to maintain this code. Not to mention that
it increases the (already great) complexity of our migration code and
adds another dimension to a testing matrix.

Jirka

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

Re: [libvirt] [PATCH 0/3] several cgroups/cpuset fixes

[John Ferlan]

On 01/07/2016 02:01 PM, Henning Schild wrote:
> On Thu, 7 Jan 2016 11:20:23 -0500
> John Ferlan  wrote:
> 
>>
>> [...]
>>
 No problem - although it seems they've generated a regression in
 the virttest memtune test suite.  I'm 'technically' on vacation
 for the next couple of weeks; however, I think/perhaps the problem
 is a result of this patch and the change to adding the task to the
 cgroup at the end of the for loop, but perhaps the following code
 causes the control to jump back to the top of the loop:

  if (!cpumap)
  continue;

   if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) <
 0) goto cleanup;

 not allowing the


 /* move the thread for vcpu to sub dir */
 if (virCgroupAddTask(cgroup_vcpu,
  qemuDomainGetVcpuPid(vm, i)) < 0)
 goto cleanup;

 to be executed.

 The code should probably change to be (like IOThreads):

  if (cpumap &&
  qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) <
 0) goto cleanup;


 As for the rest, I suspect things will be quite quiet around here
 over the next couple of weeks. A discussion to perhaps start in
 the new year.
>>>
>>> Same here. I will have a look at that regression after my vacation,
>>> should it still be there.
>>>
>>> Henning
>>>
>>
>> More data from the issue...  While the above mentioned path is an
>> issue, I don't believe it's what's causing the test failure.
>>
>> I haven't quite figured out why yet, but it seems the /proc/#/cgroup
>> file isn't getting the proper path for the 'memory' slice and thus the
>> test fails because it's looking at the:
>>
>>/sys/fs/cgroup/memory/machine.slice/memory.*
>>
>> files instead of the
>>
>> /sys/fs/cgroup/memory/machine.slice/$path/memory.*
> 
> To be honest i did just look at the cgroup/cpuset/ hierarchy, but i
> just browsed cgroup/memory/ as well.
> 
> The target of my patch series was to get
> cgroup/cpuset/machine.slice/tasks to be emtpy, all tasks should be in
> their sub-cgroup under the machine.slice. And the ordering patches make
> sure the file is always empty.
> 
> In the memory cgroups all tasks are in the parent group (all in
> machine.slice/tasks). machine.slice/*/tasks are empty. I am not sure
> whether that is intended, i can just assume it is a bug in the memory
> cgroup subsystem. Why are the groups created and tuned when the tasks
> stay in the big superset?

TBH - there's quite a bit of this that mystifies me... Use of cgroups is
not something I've spent a whole lot of time looking at...

I guess I've been working under the assumption that when the
machine.slice/$path is created, the domain would use that for all cgroup
specific file adjustments for that domain. Not sure how the
/proc/$pid/cgroup is related to this.

My f23 system seems to generate the /proc/$pid/cgroup with the
machine.slice/$path/ for each of the cgroups libvirt cares about while
the f20 system with the test only has that path for cpuset and
cpu,cpuacct. Since that's what the test uses for to find the memory path
for validation that's why it fails.

I've been looking through the libvirtd debug logs to see if anything
jumps out at me, but it seems both the systems I've looked at will build
the path for the domain using the machine.slice/$path as seen during
domain startup.

Very odd - perhaps looking at it too long right now though!


> /proc/#/cgroup is showing the correct path, libvirt seems to fail to
> migrate tasks into memory subgroups. (i am talking about a patched
> 1.2.19 where vms do not have any special memory tuning)

I'm using latest upstream 1.3.1 - it seems to set the
machine.slice/$path for blkio, cpu,cpuacct, cpuset, memory, and devices
entries.

> 
> Without my patches the first qemu thread was in
> "2:cpuset:/machine.slice" and the name did match
> "4:memory:/machine.slice". Now if the test wants matching names the
> test might just be wrong. Or as indicated before there might be a bug
> in the memory cgroups.
> 

I'm leaning towards something in the test. I'll check if reverting these
changes alters the results. I don't imagine it will.

John
>> Where $path is "machine-qemu\x2dvirt\x2dtests\x2dvm1.scope"
>>
>> This affects the virsh memtune $dom command test suite which uses the
>> /proc/$pid/cgroup file in order to find the path for the 'memory' or
>> 'cpuset' or 'cpu,cpuacct' cgroup paths.
>>
>> Seems to be some interaction with systemd that I have quite figured
>> out.
>>
>> I'm assuming this is essentially the issue you were trying to fix -
>> that is changes to values should be done to the machine-qemu*
>> specific files rather than the machine.slice files.
>>
>> The good news is I can see the changes occurring in the machine-qemu*
>> specific files, so it seems libvirt is doing the right thing.
>>
>> However, 

[libvirt] [PATCH] vmx: Adapt to emptyBackingString for cdrom-image

[Michal Privoznik]

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

We are missing this value for cdrom-image device. Honestly, I
have no idea whether it can happen for other disk devices too.

Signed-off-by: Michal Privoznik 
---
 src/vmx/vmx.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 568b2c7..baf27de 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2317,6 +2317,16 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
  */
 goto ignore;
 }
+} else if (STREQ(fileName, "emptyBackingString")) {
+if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Expecting VMX entry '%s' to be 'cdrom-image' 
"
+ "but found '%s'"), deviceType_name, 
deviceType);
+goto cleanup;
+}
+
+virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
+ignore_value(virDomainDiskSetSource(*def, NULL));
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid or not yet handled value '%s' "
-- 
2.4.10

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

Re: [libvirt] [PATCH] qemu: Don't bother user with libvirt-internal paths

[Martin Kletzander]

On Thu, Jan 07, 2016 at 10:47:46AM +0100, Pavel Hrdina wrote:

On Thu, Jan 07, 2016 at 10:18:26AM +0100, Martin Kletzander wrote:

On Thu, Jan 07, 2016 at 09:40:29AM +0100, Pavel Hrdina wrote:
>On Wed, Jan 06, 2016 at 05:50:57PM +0100, Martin Kletzander wrote:
>> If user defines a virtio channel with UNIX socket backend and doesn't
>> care about the path for the socket (e.g. qemu-agent channel), we still
>> generate it into the persistent XML.  Moreover when then user renames
>> the domain, due to its persistent socket path saved into the per-domain
>> directory, it will not start.  So let's forget about old generated paths
>> and also stop putting them into the persistent definition.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1278068
>>
>> Signed-off-by: Martin Kletzander 
>> ---
>>  src/qemu/qemu_command.c | 12 
>>  src/qemu/qemu_domain.c  | 21 
+++--
>>  .../qemuxml2argv-channel-virtio-unix.args   |  5 -
>>  .../qemuxml2argv-channel-virtio-unix.xml|  4 
>>  4 files changed, 31 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 66ca11152ad8..c5127cfa04f9 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -10510,6 +10510,18 @@ qemuBuildCommandLine(virConnectPtr conn,
>>  goto error;
>>  }
>>
>> +if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>> +!channel->source.data.nix.path) {
>> +if (virAsprintf(>source.data.nix.path,
>> +"%s/domain-%s/%s",
>> +cfg->channelTargetDir, def->name,
>> +channel->target.name ? channel->target.name
>> +: "unknown.sock") < 0)
>> +goto error;
>> +
>> +channel->source.data.nix.listen = true;
>> +}
>> +
>
>I don't like this.  The qemuBuildCommandLine function should only create a
>command line, not modify the domain definition.  I know, there are other 
places,
>that do the same, but let's try to avoid it.
>

But it's the only function that gets involved from the tests.  We should
have different places for generating stuff and formatting command line,
I know, it looks awful.  But for now it fixes the issue and I wasn't
sure whether Jiri wasn't messing with any other
qemuProcess{Init,Launch,Start,Whatever} renames and rebuilds.  Is there
any place now that we could use without refactoring the whole thing?


Without refactoring, probably not.  Jiri finished is work with qemuProcess*
functions.  I'm not saying to refactor it in scope of this patch, I just wanted
to take a minute and point this out.  You can use the similar way like in ma
patch for this issue or just add a TODO comment and refactor it later.



OK, I added the TODO: and pushed this.  You are right, we should do
something about this, but this is not the right place to do it.
Moreover, there are other things as well that should be part of the code
movement, so it needs more decisions from other people as well.

Thanks for the review.



>>  if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) &&
>>  channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
>>  /* spicevmc was originally introduced via a -device
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>
>[...]
>
>Otherwise it looks good.
>
>Pavel





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

[libvirt] [PATCH 1/2] virDomainMigrateUnmanagedParams: Don't blindly dereference @dconnuri

[Michal Privoznik]

This function may be called with @dconnuri == NULL, e.g. from
virDomainMigrateToURI3() if the flags are missing
VIR_MIGRATE_PEER2PEER flag. Moreover, all later functions called
from here do wrap it into NULLSTR() so why not do the same here?

Signed-off-by: Michal Privoznik 
---
 src/libvirt-domain.c  | 2 +-
 src/qemu/qemu_migration.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7290892..677a9ad 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3428,7 +3428,7 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
 unsigned int flags)
 {
 VIR_DOMAIN_DEBUG(domain, "dconnuri=%s, params=%p, nparams=%d, flags=%x",
- dconnuri, params, nparams, flags);
+ NULLSTR(dconnuri), params, nparams, flags);
 VIR_TYPED_PARAMS_DEBUG(params, nparams);
 
 if ((flags & VIR_MIGRATE_PEER2PEER) &&
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index bb708a3..44ca91a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5646,7 +5646,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver,
  cookieout, cookieoutlen,
  flags, resource);
 } else {
-return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri,
+return qemuMigrationPerformJob(driver, conn, vm, xmlin, NULL,
uri, graphicsuri, listenAddress,
nmigrate_disks, migrate_disks,
cookiein, cookieinlen,
-- 
2.4.10

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

Re: [libvirt] [PATCH] qemu: Don't bother user with libvirt-internal paths

[Pavel Hrdina]

On Wed, Jan 06, 2016 at 05:50:57PM +0100, Martin Kletzander wrote:
> If user defines a virtio channel with UNIX socket backend and doesn't
> care about the path for the socket (e.g. qemu-agent channel), we still
> generate it into the persistent XML.  Moreover when then user renames
> the domain, due to its persistent socket path saved into the per-domain
> directory, it will not start.  So let's forget about old generated paths
> and also stop putting them into the persistent definition.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1278068
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_command.c | 12 
>  src/qemu/qemu_domain.c  | 21 
> +++--
>  .../qemuxml2argv-channel-virtio-unix.args   |  5 -
>  .../qemuxml2argv-channel-virtio-unix.xml|  4 
>  4 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 66ca11152ad8..c5127cfa04f9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -10510,6 +10510,18 @@ qemuBuildCommandLine(virConnectPtr conn,
>  goto error;
>  }
> 
> +if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> +!channel->source.data.nix.path) {
> +if (virAsprintf(>source.data.nix.path,
> +"%s/domain-%s/%s",
> +cfg->channelTargetDir, def->name,
> +channel->target.name ? channel->target.name
> +: "unknown.sock") < 0)
> +goto error;
> +
> +channel->source.data.nix.listen = true;
> +}
> +

I don't like this.  The qemuBuildCommandLine function should only create a
command line, not modify the domain definition.  I know, there are other places,
that do the same, but let's try to avoid it.

>  if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) &&
>  channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
>  /* spicevmc was originally introduced via a -device
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c

[...]

Otherwise it looks good.

Pavel

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

[libvirt] [PATCH] Fix LSB requirements in service script and sync them

[Martin Kletzander]

Commit b22344f3285187ee1768d6e031bc0ff20e32552d mistakenly reordered
Default-* lines.  Thanks to that I noticed that we are very inconsistent
with our init scripts, so I took the liberty of synchronizing them,
updating them and making them all look shiny and new.  So apart from
fixing the LSB requirements, I also fixed the ordering, specified
runlevels and fix the link to the reference specification.

Signed-off-by: Martin Kletzander 
---
 daemon/libvirtd.init.in   | 11 ---
 src/locking/virtlockd.init.in | 12 
 src/logging/virtlogd.init.in  |  8 
 tools/libvirt-guests.init.in  |  9 ++---
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in
index 22006c448cdf..164732905d4b 100644
--- a/daemon/libvirtd.init.in
+++ b/daemon/libvirtd.init.in
@@ -1,19 +1,16 @@
 #!/bin/sh

 # the following is the LSB init header see
-# 
http://www.linux-foundation.org/spec//booksets/LSB-Core-generic/LSB-Core-generic.html#INITSCRCOMCONV
+# 
http://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/initscrcomconv.html
 #
 ### BEGIN INIT INFO
 # Provides: libvirtd
+# Default-Start: 3 4 5
+# Default-Stop: 0 1 2 6
 # Required-Start: $network messagebus virtlogd
-# Should-Start: $named
-# Should-Start: xend
-# Should-Start: avahi-daemon
-# Should-Start: virtlockd
 # Required-Stop: $network messagebus
+# Should-Start: $named xend avahi-daemon virtlockd
 # Should-Stop: $named
-# Default-Start: 3 4 5
-# Default-Stop: 0 1 2 6
 # Short-Description: daemon for libvirt virtualization API
 # Description: This is a daemon for managing guest instances
 #  and libvirt virtual networks
diff --git a/src/locking/virtlockd.init.in b/src/locking/virtlockd.init.in
index 596bb6241485..0bf868ca7ff1 100644
--- a/src/locking/virtlockd.init.in
+++ b/src/locking/virtlockd.init.in
@@ -1,12 +1,16 @@
 #!/bin/sh

 # the following is the LSB init header see
-# 
http://www.linux-foundation.org/spec//booksets/LSB-Core-generic/LSB-Core-generic.html#INITSCRCOMCONV
+# 
http://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/initscrcomconv.html
 #
 ### BEGIN INIT INFO
 # Provides: virtlockd
-# Default-Start:
-# Default-Stop: 0 1 2 3 4 5 6
+# Default-Start: 3 4 5
+# Default-Stop: 0 1 2 6
+# Required-Start:
+# Required-Stop:
+# Should-Start: $network $remote_fs
+# Should-Stop: $network $remote_fs
 # Short-Description: virtual machine lock manager
 # Description: This is a daemon for managing locks
 #  on virtual machine disk images
@@ -16,7 +20,7 @@
 #
 # virtlockd:   virtual machine lock manager
 #
-# chkconfig: - 96 04
+# chkconfig: 345 96 04
 # description:  This is a daemon for managing locks \
 #   on virtual machine disk images
 #
diff --git a/src/logging/virtlogd.init.in b/src/logging/virtlogd.init.in
index 89b243dc0522..6aa88150469f 100644
--- a/src/logging/virtlogd.init.in
+++ b/src/logging/virtlogd.init.in
@@ -1,16 +1,16 @@
 #!/bin/sh

 # the following is the LSB init header see
-# 
http://www.linux-foundation.org/spec//booksets/LSB-Core-generic/LSB-Core-generic.html#INITSCRCOMCONV
+# 
http://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/initscrcomconv.html
 #
 ### BEGIN INIT INFO
 # Provides: virtlogd
-# Default-Start: 3 5
+# Default-Start: 3 4 5
+# Default-Stop: 0 1 2 6
 # Required-Start:
 # Required-Stop:
 # Should-Start: $network $remote_fs
 # Should-Stop: $network $remote_fs
-# Default-Stop: 0 1 2 4 6
 # Short-Description: virtual machine log manager
 # Description: This is a daemon for managing logs
 #  of virtual machine consoles
@@ -20,7 +20,7 @@
 #
 # virtlogd:   virtual machine log manager
 #
-# chkconfig: - 96 04
+# chkconfig: 345 96 04
 # description:  This is a daemon for managing logs \
 #   of virtual machine consoles
 #
diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.in
index 5f9a60e81f05..7709df3b96a3 100644
--- a/tools/libvirt-guests.init.in
+++ b/tools/libvirt-guests.init.in
@@ -1,13 +1,16 @@
 #!/bin/sh

-# the following is the LSB init header
+# the following is the LSB init header see
+# 
http://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/initscrcomconv.html
 #
 ### BEGIN INIT INFO
 # Provides: libvirt-guests
+# Default-Start: 3 4 5
+# Default-Stop: 0 1 2 6
 # Required-Start: libvirtd
 # Required-Stop: libvirtd
-# Default-Start: 2 3 4 5
-# Default-Stop: 0 1 6
+# Should-Start:
+# Should-Stop:
 # Short-Description: suspend/resume libvirt guests on shutdown/boot
 # Description: This is a script for suspending active libvirt guests
 #  on shutdown and resuming them on next boot
-- 
2.7.0

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

[libvirt] [PATCH 07/10] tests: regenerate all outputs using VIR_TEST_REGENERATE_OUTPUT flag

[Pavel Hrdina]

Since more tests supports regenerating its output, this patch updates
those expected output files.

Signed-off-by: Pavel Hrdina 
---
 tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-cdrom.xml   | 5 +++--
 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-disk.xml| 5 +++--
 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-v2-none.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-policy.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-websocket.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-kvm-features.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-argv.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-argv.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-argv.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-argv.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none-argv.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-nosharepages.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-disabled.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-enabled.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml   | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml| 4 ++--
 45 files changed, 91 insertions(+), 89 deletions(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml
index b639821..d36e9a9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml
@@ -1,8 +1,8 @@
 
   QEMUGuest1
   c7a5fdbd-edaf-9455-926a-d65c16db1809
-  219100
-  219100
+  219136
+  219136
   1
   
 hvm
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml
index 610321f..e2dfa6d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml
@@ -1,8 +1,8 @@
 
   QEMUGuest1
   c7a5fdbd-edaf-9455-926a-d65c16db1809
-  219100
-  219100
+  219136
+  219136
   1
   
 hvm
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml
index f4ebc2e..58c98f1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml
@@ -1,8 +1,8 @@
 
   QEMUGuest1
   c7a5fdbd-edaf-9455-926a-d65c16db1809
-  219100
-  219100
+  219136
+  219136
   1
   
 hvm
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml 

[libvirt] [PATCH 08/10] tests.nwfilterebiptablestest: swap actual and expected

[Pavel Hrdina]

Those parameters should be in opposite order.

Signed-off-by: Pavel Hrdina 
---
 tests/nwfilterebiptablestest.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index e1330ef..6bd6aad 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -116,7 +116,7 @@ testNWFilterEBIPTablesAllTeardown(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -187,7 +187,7 @@ testNWFilterEBIPTablesTearOldRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -236,7 +236,7 @@ testNWFilterEBIPTablesRemoveBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -270,7 +270,7 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -342,7 +342,7 @@ testNWFilterEBIPTablesApplyBasicRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -432,7 +432,7 @@ testNWFilterEBIPTablesApplyDHCPOnlyRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
@@ -505,7 +505,7 @@ testNWFilterEBIPTablesApplyDropAllRules(const void *opaque 
ATTRIBUTE_UNUSED)
 virtTestClearCommandPath(actual);
 
 if (STRNEQ_NULLABLE(actual, expected)) {
-virtTestDifference(stderr, actual, expected);
+virtTestDifference(stderr, expected, actual);
 goto cleanup;
 }
 
-- 
2.7.0

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

[libvirt] [PATCH 00/10] [rebase] various test fixes and input devices handling

[Pavel Hrdina]

This is just a rebase to current HEAD.

Pavel Hrdina (10):
  xen: move virDomainDefPostParse to xenParseSxpr
  tests: introduce VIR_TEST_REGENERATE_OUTPUT flag
  tests.testutils: use VIR_TEST_REGENERATE_OUTPUT for
virTestDifferenceFull
  tests.testutils: use virTestDifferenceFull in virtTestCompareToFile
  tests: use virtTestDifferenceFull in tests where we have output file
  tests: skip regeneration for problematic test
  tests: regenerate all outputs using VIR_TEST_REGENERATE_OUTPUT flag
  tests.nwfilterebiptablestest: swap actual and expected
  device: cleanup input device code
  tests: update test XML files to follow input changes

 src/Makefile.am|   4 +-
 src/conf/domain_conf.c |  77 +---
 src/libxl/libxl_domain.c   |   5 +
 src/qemu/qemu_domain.c |  22 
 src/xen/xen_driver.c   |   5 +
 src/xen/xend_internal.c|   7 +-
 src/xenapi/xenapi_driver.c |   5 +
 src/xenconfig/xen_common.c |  22 
 src/xenconfig/xen_common.h |   2 +
 src/xenconfig/xen_sxpr.c   |  21 ++--
 src/xenconfig/xen_sxpr.h   |   6 +-
 .../disk_snapshot_redefine.xml |   2 +
 .../external_vm_redefine.xml   |   2 +
 tests/domainsnapshotxml2xmlout/full_domain.xml |   2 +
 tests/domainsnapshotxml2xmlout/metadata.xml|   2 +
 tests/domainsnapshotxml2xmltest.c  |   2 +-
 tests/interfacexml2xmltest.c   |   2 +-
 tests/lxcconf2xmltest.c|   2 +-
 tests/nodedevxml2xmltest.c |   2 +-
 tests/nwfilterebiptablestest.c |  14 +--
 tests/qemuargv2xmltest.c   |  18 ++-
 tests/qemuhotplugtest.c|  11 +-
 .../qemuhotplug-hotplug-base+disk-scsi.xml |   2 +
 .../qemuhotplug-hotplug-base+disk-usb.xml  |   2 +
 .../qemuhotplug-hotplug-base+disk-virtio.xml   |   2 +
 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml |   2 +
 .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml |   2 +
 .../qemuxml2argv-blkiotune-device.xml  |   2 +
 tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml  |   2 +
 tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml |   6 +-
 .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml  |   6 +-
 .../qemuxml2argv-boot-menu-disable.xml |   2 +
 .../qemuxml2argv-boot-menu-enable-with-timeout.xml |   2 +
 tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml |   2 +
 .../qemuxml2argvdata/qemuxml2argv-boot-network.xml |   6 +-
 tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml |   2 +
 .../qemuxml2argv-channel-guestfwd.xml  |   2 +
 .../qemuxml2argv-channel-virtio.xml|   2 +
 .../qemuxml2argv-chardev-label.xml |   2 +
 .../qemuxml2argv-clock-catchup.xml |   2 +
 .../qemuxml2argv-clock-localtime.xml   |   6 +-
 .../qemuxml2argv-clock-timer-hyperv-rtc.xml|   2 +
 tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml  |   6 +-
 .../qemuxml2argv-console-compat.xml|   6 +-
 .../qemuxml2argv-console-virtio-many.xml   |   2 +
 .../qemuxml2argv-cpu-eoi-disabled.xml  |   2 +
 .../qemuxml2argv-cpu-eoi-enabled.xml   |   2 +
 .../qemuxml2argv-cpu-host-kvmclock.xml |   2 +
 .../qemuxml2argv-cpu-host-model-features.xml   |   2 +
 .../qemuxml2argv-cpu-host-passthrough-features.xml |   2 +
 .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml |   2 +
 .../qemuxml2argv-cpu-numa-disjoint.xml |   2 +
 .../qemuxml2argv-cpu-numa-memshared.xml|   2 +
 ...xml2argv-cputune-iothreadsched-zeropriority.xml |   2 +
 .../qemuxml2argv-cputune-numatune.xml  |   2 +
 .../qemuxml2argv-cputune-zero-shares.xml   |   2 +
 tests/qemuxml2argvdata/qemuxml2argv-cputune.xml|   2 +
 .../qemuxml2argv-disk-active-commit.xml|   2 +
 tests/qemuxml2argvdata/qemuxml2argv-disk-aio.xml   |   2 +
 .../qemuxml2argv-disk-cdrom-empty.xml  |   6 +-
 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml |   6 +-
 .../qemuxml2argv-disk-copy_on_read.xml |   2 +
 .../qemuxml2argv-disk-drive-boot-cdrom.xml |   7 +-
 .../qemuxml2argv-disk-drive-boot-disk.xml  |   7 +-
 .../qemuxml2argv-disk-drive-cache-directsync.xml   |   2 +
 .../qemuxml2argv-disk-drive-cache-unsafe.xml   |   2 +
 .../qemuxml2argv-disk-drive-cache-v2-none.xml  |   6 +-
 .../qemuxml2argv-disk-drive-cache-v2-wb.xml|   2 +
 .../qemuxml2argv-disk-drive-cache-v2-wt.xml|   2 +
 .../qemuxml2argv-disk-drive-copy-on-read.xml   |   2 +
 ...muxml2argv-disk-drive-error-policy-enospace.xml |   2 +
 

[libvirt] [PATCH 01/10] xen: move virDomainDefPostParse to xenParseSxpr

[Pavel Hrdina]

This patch partially reverts previous commit 91a00424 and moves the post
parse function to xenParseSxpr.  This update is required because xen
driver calls xenParseSxpr directly.

Signed-off-by: Pavel Hrdina 
---
 src/xen/xend_internal.c  |  7 +--
 src/xenconfig/xen_sxpr.c | 21 ++---
 src/xenconfig/xen_sxpr.h |  6 +-
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index db3820d..cf7cdd0 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1552,7 +1552,9 @@ xenDaemonDomainFetch(virConnectPtr conn, int domid, const 
char *name,
 if (!(def = xenParseSxpr(root,
  cpus,
  tty,
- vncport)))
+ vncport,
+ priv->caps,
+ priv->xmlopt)))
 goto cleanup;
 
  cleanup:
@@ -3082,7 +3084,8 @@ xenDaemonDomainBlockPeek(virConnectPtr conn,
 vncport = xenStoreDomainGetVNCPort(conn, id);
 xenUnifiedUnlock(priv);
 
-if (!(def = xenParseSxpr(root, NULL, tty, vncport)))
+if (!(def = xenParseSxpr(root, NULL, tty, vncport,
+ priv->caps, priv->xmlopt)))
 goto cleanup;
 
 if (!(actual = virDomainDiskPathByName(def, path))) {
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index d99bac0..d7b700d 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -1065,7 +1065,11 @@ xenParseSxprPCI(virDomainDefPtr def,
  */
 virDomainDefPtr
 xenParseSxpr(const struct sexpr *root,
- const char *cpus, char *tty, int vncport)
+ const char *cpus,
+ char *tty,
+ int vncport,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt)
 {
 const char *tmp;
 virDomainDefPtr def;
@@ -1371,6 +1375,10 @@ xenParseSxpr(const struct sexpr *root,
 goto error;
 }
 
+if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+  xmlopt) < 0)
+goto error;
+
 return def;
 
  error:
@@ -1405,16 +1413,7 @@ xenParseSxprString(const char *sexpr,
 if (!root)
 return NULL;
 
-if (!(def = xenParseSxpr(root, NULL, tty, vncport)))
-goto cleanup;
-
-if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
-  xmlopt) < 0) {
-virDomainDefFree(def);
-def = NULL;
-}
-
- cleanup:
+def = xenParseSxpr(root, NULL, tty, vncport, caps, xmlopt);
 sexpr_free(root);
 
 return def;
diff --git a/src/xenconfig/xen_sxpr.h b/src/xenconfig/xen_sxpr.h
index a4f4c44..e925208 100644
--- a/src/xenconfig/xen_sxpr.h
+++ b/src/xenconfig/xen_sxpr.h
@@ -43,7 +43,11 @@ virDomainDefPtr xenParseSxprString(const char *sexpr,
virDomainXMLOptionPtr xmlopt);
 
 virDomainDefPtr xenParseSxpr(const struct sexpr *root,
- const char *cpus, char *tty, int vncport);
+ const char *cpus,
+ char *tty,
+ int vncport,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt);
 
 int xenParseSxprSound(virDomainDefPtr def, const char *str);
 
-- 
2.7.0

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

[libvirt] [PATCH 05/10] tests: use virtTestDifferenceFull in tests where we have output file

[Pavel Hrdina]

This will enable regenerate functionality for those tests to make
developer lives easier while updating tests.

Signed-off-by: Pavel Hrdina 
---
 tests/domainsnapshotxml2xmltest.c |  2 +-
 tests/interfacexml2xmltest.c  |  2 +-
 tests/lxcconf2xmltest.c   |  2 +-
 tests/nodedevxml2xmltest.c|  2 +-
 tests/qemuargv2xmltest.c  |  7 ---
 tests/qemuhotplugtest.c   | 11 ---
 tests/vboxsnapshotxmltest.c   |  2 +-
 7 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tests/domainsnapshotxml2xmltest.c 
b/tests/domainsnapshotxml2xmltest.c
index cd91cfa..cf91447 100644
--- a/tests/domainsnapshotxml2xmltest.c
+++ b/tests/domainsnapshotxml2xmltest.c
@@ -114,7 +114,7 @@ testCompareXMLToXMLFiles(const char *inxml,
 }
 
 if (STRNEQ(outXmlData, actual)) {
-virtTestDifference(stderr, outXmlData, actual);
+virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml);
 goto cleanup;
 }
 
diff --git a/tests/interfacexml2xmltest.c b/tests/interfacexml2xmltest.c
index 65f5167..ba34746 100644
--- a/tests/interfacexml2xmltest.c
+++ b/tests/interfacexml2xmltest.c
@@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *xml)
 goto fail;
 
 if (STRNEQ(xmlData, actual)) {
-virtTestDifference(stderr, xmlData, actual);
+virtTestDifferenceFull(stderr, xmlData, xml, actual, NULL);
 goto fail;
 }
 
diff --git a/tests/lxcconf2xmltest.c b/tests/lxcconf2xmltest.c
index ed21e8a..fd5bc03 100644
--- a/tests/lxcconf2xmltest.c
+++ b/tests/lxcconf2xmltest.c
@@ -51,7 +51,7 @@ testCompareXMLToConfigFiles(const char *xml,
 goto fail;
 
 if (STRNEQ(expectxml, actualxml)) {
-virtTestDifference(stderr, expectxml, actualxml);
+virtTestDifferenceFull(stderr, expectxml, xml, actualxml, NULL);
 goto fail;
 }
 }
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index a37d729..0089b5d 100644
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *xml)
 goto fail;
 
 if (STRNEQ(xmlData, actual)) {
-virtTestDifference(stderr, xmlData, actual);
+virtTestDifferenceFull(stderr, xmlData, xml, actual, NULL);
 goto fail;
 }
 
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 7759a09..fc18b55 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -90,12 +90,13 @@ static int testCompareXMLToArgvFiles(const char *xml,
 if (!(actualxml = virDomainDefFormat(vmdef, 0)))
 goto fail;
 
-if (blankProblemElements(expectxml) < 0 ||
-blankProblemElements(actualxml) < 0)
+if (!virTestGetRegenerate() &&
+(blankProblemElements(expectxml) < 0 ||
+ blankProblemElements(actualxml) < 0))
 goto fail;
 
 if (STRNEQ(expectxml, actualxml)) {
-virtTestDifference(stderr, expectxml, actualxml);
+virtTestDifferenceFull(stderr, expectxml, xml, actualxml, NULL);
 goto fail;
 }
 
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 102e052..61ade25 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -175,6 +175,7 @@ testQemuHotplugUpdate(virDomainObjPtr vm,
 static int
 testQemuHotplugCheckResult(virDomainObjPtr vm,
const char *expected,
+   const char *expectedFile,
bool fail)
 {
 char *actual;
@@ -192,7 +193,9 @@ testQemuHotplugCheckResult(virDomainObjPtr vm,
 ret = 0;
 } else {
 if (!fail)
-virtTestDifference(stderr, expected, actual);
+virtTestDifferenceFull(stderr,
+   expected, expectedFile,
+   actual, NULL);
 ret = -1;
 }
 
@@ -294,13 +297,15 @@ testQemuHotplug(const void *data)
 VIR_FREE(dev);
 }
 if (ret == 0 || fail)
-ret = testQemuHotplugCheckResult(vm, result_xml, fail);
+ret = testQemuHotplugCheckResult(vm, result_xml,
+ result_filename, fail);
 break;
 
 case DETACH:
 ret = testQemuHotplugDetach(vm, dev);
 if (ret == 0 || fail)
-ret = testQemuHotplugCheckResult(vm, domain_xml, fail);
+ret = testQemuHotplugCheckResult(vm, domain_xml,
+ domain_filename, fail);
 break;
 
 case UPDATE:
diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
index 4ee7537..87b8571 100644
--- a/tests/vboxsnapshotxmltest.c
+++ b/tests/vboxsnapshotxmltest.c
@@ -85,7 +85,7 @@ testCompareXMLtoXMLFiles(const char *xml)
 goto cleanup;
 
 if (STRNEQ(actual, xmlData)) {
-virtTestDifference(stderr, xmlData, actual);
+virtTestDifferenceFull(stderr, xmlData, xml, actual, 

[libvirt] [PATCH 02/10] tests: introduce VIR_TEST_REGENERATE_OUTPUT flag

[Pavel Hrdina]

When this flag is specified, some of the expected output files will be
regenerated with the actual output data.  This is a helper for updating
test data.

Signed-off-by: Pavel Hrdina 
---
 tests/testutils.c | 14 +++---
 tests/testutils.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index 857e819..2b0d3b6 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -67,6 +67,7 @@ VIR_LOG_INIT("tests.testutils");
 static unsigned int testDebug = -1;
 static unsigned int testVerbose = -1;
 static unsigned int testExpensive = -1;
+static unsigned int testRegenerate = -1;
 
 #ifdef TEST_OOM
 static unsigned int testOOM;
@@ -598,9 +599,8 @@ virtTestCompareToFile(const char *strcontent,
 int ret = -1;
 char *filecontent = NULL;
 char *fixedcontent = NULL;
-bool regenerate = !!virTestGetFlag("VIR_TEST_REGENERATE_OUTPUT");
 
-if (virtTestLoadFile(filename, ) < 0 && !regenerate)
+if (virtTestLoadFile(filename, ) < 0 && 
!virTestGetRegenerate())
 goto failure;
 
 if (filecontent &&
@@ -612,7 +612,7 @@ virtTestCompareToFile(const char *strcontent,
 
 if (STRNEQ_NULLABLE(fixedcontent ? fixedcontent : strcontent,
 filecontent)) {
-if (regenerate) {
+if (virTestGetRegenerate()) {
 if (virFileWriteStr(filename, strcontent, 0666) < 0)
 goto failure;
 goto out;
@@ -716,6 +716,14 @@ virTestGetExpensive(void)
 return testExpensive;
 }
 
+unsigned int
+virTestGetRegenerate(void)
+{
+if (testRegenerate == -1)
+testRegenerate = virTestGetFlag("VIR_TEST_REGENERATE_OUTPUT");
+return testRegenerate;
+}
+
 int virtTestMain(int argc,
  char **argv,
  int (*func)(void))
diff --git a/tests/testutils.h b/tests/testutils.h
index ccf1d29..8ef70e4 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -77,6 +77,7 @@ int virtTestCompareToFile(const char *strcontent,
 unsigned int virTestGetDebug(void);
 unsigned int virTestGetVerbose(void);
 unsigned int virTestGetExpensive(void);
+unsigned int virTestGetRegenerate(void);
 
 # define VIR_TEST_DEBUG(...)\
 do {\
-- 
2.7.0

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

Re: [libvirt] [PATCH 1/2] xenconfig: support parsing and formatting vif bandwidth

[Michal Privoznik]

On 29.12.2015 02:09, Jim Fehlig wrote:
> Both xm and xl config have long supported specifying vif rate
> limiting, e.g.
> 
> vif = [ 'mac=00:16:3E:74:3d:76,bridge=br0,rate=10MB/s' ]
> 
> Add support for mapping rate to and from  in the xenconfig
> parser and formatter. rate is mapped to the required 'average' attribute
> of the  element, e.g.
> 
>   
> ...
> 
>   
> 
>   
> 
> Also add a unit test to check the conversion logic.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> I used a bit of code from libxlu_vif.c to implement xenParseVifRate()
> instead of using the libxlutil lib directly, since in theory rate limiting
> applies to the old xen driver (no libxl) as well.
> 
>  src/xenconfig/xen_common.c   | 77 
> 
>  tests/xlconfigdata/test-vif-rate.cfg | 26 
>  tests/xlconfigdata/test-vif-rate.xml | 57 ++
>  tests/xlconfigtest.c |  1 +
>  4 files changed, 161 insertions(+)

ACK

Michal

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

Re: [libvirt] [PATCH v2 00/14] Use macros for more common virsh command options

[John Ferlan]

On 01/06/2016 01:58 PM, Laine Stump wrote:
> On 12/18/2015 10:45 AM, John Ferlan wrote:
>> v1:
>> http://www.redhat.com/archives/libvir-list/2015-December/msg00731.html
>>
>> Changes over v1:
>>
>>   1. Insert patch 1 to convert already pushed VSH_POOL into VIRSH_POOL
>>  since that was the review comment from this patch series
>>
>>   2. Insert patch 2 to move the POOL_OPT_COMMON to virsh.h for later
>>  patch reuse.
>>
>>   3. Use VIRSH_* instead of VSH_* for patches 1-8 (now 3-10)
>>
>>   4. Add usage of common domain for virsh-domain-monitor.c and
>>  virsh-snapshot.c (patches 11-12)
>>
>>   5. Add common macros for "network" and "interface" (patches 13-14).
>>
>> NOTE: I figure to let this perculate for a bit as I'll assume there
>> may be varying opinions on this...  Also, the next couple of weeks
>> heavy on people perhaps paying not paying close attention to the list.
> 
> I'm a bit conflicted. On one hand, it makes for less bulk in the code
> and assures consistency when the same option is used by multiple
> commands. On the other hand, as Peter said in a response to the original
> patch, it obscures things behind macros which can lead to confusion (and
> extra time backtracking).

Understood; however, at least they're more or less easily found
especially if you use cscope. There are certainly some macros in the
source code which are far more obfuscated!

> 
> If you're looking for a final "vote" from me, I guess I'd give it +1
> (brevity wins this time), with these comments:
> 
> 1) I assume that make check and make syntax-check have been run for each
> patch. :-)
> 

yes, painfully ;-)

> 2) I agree with using "VIRSH_..." instead of "VSH_..." (I dislike the
> shortening of virsh to vsh - doing that just for 2 characters? What is
> this, twitter? :-P)
> 

Newsflash - twitter is apparently expanding to allow 1 characters.
Maybe now I'll be able to start tweeting (yeah, right, not!)

> 3) I haven't looked at how is meshes with consistency of other macro
> names in virsh*, but it would make more sense to me if these were named
> 
>VIRSH_COMMON_OPT_BLAH
> 
> instead of
> 
>VIRSH_BLAH_OPT_COMMON
> 
> It reads better, and sticks the difference out at the end where it is
> more easily separated from the "common common" part.


I was following Peter's suggested naming:

http://www.redhat.com/archives/libvir-list/2015-December/msg00675.html

but I have no favorite... If others chime in and agree, then I'm fine
with switching.

thanks for the comments -

John
>>
>> John Ferlan (14):
>>virsh: Covert VSH_POOL_ macro to VIRSH_POOL_
>>virsh: Move VIRSH_POOL_OPT_COMMON to virsh.h
>>virsh: Create macro for common "domain" option
>>virsh: Create macro for common "persistent" option
>>virsh: Create macro for common "config" option
>>virsh: Create macro for common "live" option
>>virsh: Create macro for common "current" option
>>virsh: Create macro for common "file" option
>>virsh: Create macros for common "pool" options
>>virsh: Create macros for common "vol" options
>>virsh: Have domain-monitor use common "domain" option
>>virsh: have snapshot use common "domain" option
>>virsh: Create macro for common "network" option
>>virsh: Create macro for common "interface" option
>>
>>   po/POTFILES.in   |   1 +
>>   tools/virsh-domain-monitor.c |  77 +---
>>   tools/virsh-domain.c | 911
>> +--
>>   tools/virsh-interface.c  |  37 +-
>>   tools/virsh-network.c|  61 +--
>>   tools/virsh-pool.c   |  71 ++--
>>   tools/virsh-snapshot.c   |  60 +--
>>   tools/virsh-volume.c | 148 ++-
>>   tools/virsh.h|  17 +
>>   9 files changed, 334 insertions(+), 1049 deletions(-)
>>
> 

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

Re: [libvirt] Libvirt 1.3.0 compilation failure on OSX 10.9.5

[Roman Bogorodskiy]

  Justin Clift wrote:

> On 6 Jan 2016, at 18:03, Roman Bogorodskiy  wrote:
> >  Roman Bogorodskiy wrote:
> >>  Justin Clift wrote:
> >>> Hiyas,
> >>> 
> >>> Just went to update the MacOS X Homebrew port/formula for
> >>> Libvirt 1.3.0.  It's failing during compilation with:
> >>> 
> >>> ***
> >>> 
> >>>  Last 15 lines from /Users/jc/Library/Logs/Homebrew/libvirt/02.make:
> >>>CCLD libvirt_driver_vbox.la
> >>>CCLD libvirt_driver_storage.la
> >>>  Undefined symbols for architecture x86_64:
> >>>"_xdr_uint64_t", referenced from:
> >>>_xdr_virLogManagerProtocolLogFilePosition in 
> >>> virtlogd-log_protocol.o
> >>>_xdr_virLogManagerProtocolDomainOpenLogFileRet in 
> >>> virtlogd-log_protocol.o
> >>>_xdr_virLogManagerProtocolDomainGetLogFilePositionRet in 
> >>> virtlogd-log_protocol.o
> >>>_xdr_virLogManagerProtocolDomainReadLogFileArgs in 
> >>> virtlogd-log_protocol.o
> >>>  ld: symbol(s) not found for architecture x86_64
> >>>  clang: error: linker command failed with exit code 1 (use -v to see 
> >>> invocation)
> >>>  make[3]: *** [virtlogd] Error 1
> >>>  make[3]: *** Waiting for unfinished jobs
> >>>  make[2]: *** [all] Error 2
> >>>  make[1]: *** [all-recursive] Error 1
> >>>  make: *** [all] Error 2
> >>> 
> >>> ***
> >>> 
> >>> Is this a known thing, or should I grab the rest of the failure logging 
> >>> for
> >>> someone (not me) to look at? :)
> >>> 
> >> Hi Justin,
> >> 
> >> Could you check if this patche solves the problem for you:
> >> 
> >> http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/sysutils/libvirt/patches/patch-src_admin_admin_protocol_c?rev=1.2=text/x-cvsweb-markup
> >> ?
> > 
> > Urgh, that's the wrong one. The right one is here:
> > 
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/sysutils/libvirt/patches/patch-src_logging_log_protocol_c?rev=1.2=text/x-cvsweb-markup
> > 
> > But as they're identical, I'd assume you'll need the first one as well.
> 
> Interestingly, with just this one added in, libvirt now compiles fully and
> installs fine.  Didn't need to touch admin_protocol.c at all.  Maybe that's
> only conditionally compiled or something?
> 
> virsh seems to start up ok, but I didn't actually try doing anything with
> it. ;)
> 
> Should we call this "good enough for now", or is there more to do? :)
> 
> + Justin

I'll prepare patches based on those from the OpenBSD libvirt port that
address this problem and send them out for review soon, so, hopefully
we'll get it fixed.

Roman Bogorodskiy

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

[libvirt] [PATCH 10/12] tests: qemuxml2xml: Convert some fprintf to VIR_TEST_DEBUG

[Cole Robinson]

---
 tests/qemuxml2xmltest.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e3bd9c2..c0270d4 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -59,7 +59,7 @@ testXML2XMLHelper(const char *inxml,
 goto fail;
 
 if (!virDomainDefCheckABIStability(def, def)) {
-fprintf(stderr, "ABI stability check failed on %s", inxml);
+VIR_TEST_DEBUG("ABI stability check failed on %s", inxml);
 goto fail;
 }
 
@@ -151,7 +151,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
 virBufferAdd(, testStatusXMLSuffix, -1);
 
 if (!(source = virBufferContentAndReset())) {
-fprintf(stderr, "Failed to create the source XML");
+VIR_TEST_DEBUG("Failed to create the source XML");
 goto cleanup;
 }
 
@@ -163,7 +163,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
 virBufferAdd(, testStatusXMLSuffix, -1);
 
 if (!(expect = virBufferContentAndReset())) {
-fprintf(stderr, "Failed to create the expect XML");
+VIR_TEST_DEBUG("Failed to create the expect XML");
 goto cleanup;
 }
 
@@ -174,14 +174,14 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
   VIR_DOMAIN_DEF_PARSE_STATUS |
   VIR_DOMAIN_DEF_PARSE_ACTUAL_NET |
   VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES))) {
-fprintf(stderr, "Failed to parse domain status XML:\n%s", source);
+VIR_TEST_DEBUG("Failed to parse domain status XML:\n%s", source);
 goto cleanup;
 }
 
 /* format it back */
 if (!(actual = virDomainObjFormat(driver.xmlopt, obj,
   VIR_DOMAIN_DEF_FORMAT_SECURE))) {
-fprintf(stderr, "Failed to format domain status XML");
+VIR_TEST_DEBUG("Failed to format domain status XML");
 goto cleanup;
 }
 
@@ -303,7 +303,7 @@ mymain(void)
 # define DO_TEST_FULL(name, is_different, when)
\
 do {   
\
 if (testInfoSet(, name, is_different, when) < 0) {
\
-fprintf(stderr, "Failed to generate test data for '%s'", name);
\
+VIR_TEST_DEBUG("Failed to generate test data for '%s'", name);\
 return -1; 
\
 }  
\

\
-- 
2.5.0

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

[libvirt] [PATCH] ruby-libvirt: Don't crash in leases_wrap() by passing NULLs to rb_str_new2()

[Dan Williams]

Not all lease values are mandatory, and when they aren't supplied
by the libvirt driver they get set to NULL.  That makes
rb_str_new2() bail out.

Signed-off-by: Dan Williams 
---
For example using the qemu driver we don't get 'iaid', and that
makes ruby unhappy:

[{"iface"=>"virbr1", "expirytime"=>1452189569, "type"=>0, 
"mac"=>"52:54:00:05:2b:6f",
"ipaddr"=>"192.168.121.49", "prefix"=>24, "hostname"=>"openshiftdev",
"clientid"=>"ff:00:05:2b:6f:00:01:00:01:1e:21:55:ed:52:54:00:05:2b:6f"}]

 ext/libvirt/network.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/ext/libvirt/network.c b/ext/libvirt/network.c
index 7c77d73..c250d7d 100644
--- a/ext/libvirt/network.c
+++ b/ext/libvirt/network.c
@@ -269,14 +269,20 @@ static VALUE leases_wrap(VALUE arg)
 rb_hash_aset(hash, rb_str_new2("expirytime"),
  LL2NUM(lease->expirytime));
 rb_hash_aset(hash, rb_str_new2("type"), INT2NUM(lease->type));
-rb_hash_aset(hash, rb_str_new2("mac"), rb_str_new2(lease->mac));
-rb_hash_aset(hash, rb_str_new2("iaid"), rb_str_new2(lease->iaid));
+if (lease->mac)
+rb_hash_aset(hash, rb_str_new2("mac"), rb_str_new2(lease->mac));
+if (lease->iaid)
+rb_hash_aset(hash, rb_str_new2("iaid"), rb_str_new2(lease->iaid));
 rb_hash_aset(hash, rb_str_new2("ipaddr"), rb_str_new2(lease->ipaddr));
 rb_hash_aset(hash, rb_str_new2("prefix"), UINT2NUM(lease->prefix));
-rb_hash_aset(hash, rb_str_new2("hostname"),
- rb_str_new2(lease->hostname));
-rb_hash_aset(hash, rb_str_new2("clientid"),
- rb_str_new2(lease->clientid));
+if (lease->hostname) {
+rb_hash_aset(hash, rb_str_new2("hostname"),
+ rb_str_new2(lease->hostname));
+}
+if (lease->clientid) {
+rb_hash_aset(hash, rb_str_new2("clientid"),
+ rb_str_new2(lease->clientid));
+}
 
 rb_ary_store(result, i, hash);
 }
-- 
2.4.3

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

[libvirt] [PATCH 05/12] qemu: domain: split out post parse default device handling

[Cole Robinson]

Should be a no-op
---
 src/qemu/qemu_domain.c | 53 +++---
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bb8d47f..26a29b1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1025,14 +1025,10 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = 
{
 .href = qemuDomainDefNamespaceHref,
 };
 
-
 static int
-qemuDomainDefPostParse(virDomainDefPtr def,
-   virCapsPtr caps,
-   void *opaque)
+qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
+   virQEMUCapsPtr qemuCaps)
 {
-virQEMUDriverPtr driver = opaque;
-virQEMUCapsPtr qemuCaps = NULL;
 bool addDefaultUSB = true;
 bool addImplicitSATA = false;
 bool addPCIRoot = false;
@@ -1043,20 +1039,6 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 bool addPanicDevice = false;
 int ret = -1;
 
-if (def->os.bootloader || def->os.bootloaderArgs) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("bootloader is not supported by QEMU"));
-return ret;
-}
-
-/* check for emulator and create a default one if needed */
-if (!def->emulator &&
-!(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))
-return ret;
-
-
-qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
-
 /* Add implicit PCI root controller if the machine has one */
 switch (def->os.arch) {
 case VIR_ARCH_I686:
@@ -1212,6 +1194,37 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 
 ret = 0;
  cleanup:
+return ret;
+}
+
+
+static int
+qemuDomainDefPostParse(virDomainDefPtr def,
+   virCapsPtr caps,
+   void *opaque)
+{
+virQEMUDriverPtr driver = opaque;
+virQEMUCapsPtr qemuCaps = NULL;
+int ret = -1;
+
+if (def->os.bootloader || def->os.bootloaderArgs) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("bootloader is not supported by QEMU"));
+return ret;
+}
+
+/* check for emulator and create a default one if needed */
+if (!def->emulator &&
+!(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))
+return ret;
+
+qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
+
+if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
 virObjectUnref(qemuCaps);
 return ret;
 }
-- 
2.5.0

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

[libvirt] [PATCH 02/12] domain: separate out function for post parse timer validation

[Cole Robinson]

This should be a no-op
---
 src/conf/domain_conf.c | 53 +-
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ab22322..2570f94 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3765,31 +3765,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def)
 }
 
 static int
-virDomainDefPostParseInternal(virDomainDefPtr def,
-  virCapsPtr caps ATTRIBUTE_UNUSED,
-  unsigned int parseFlags)
+virDomainDefPostParseTimer(virDomainDefPtr def)
 {
 size_t i;
 
-/* verify init path for container based domains */
-if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("init binary must be specified"));
-return -1;
-}
-
-if (virDomainDefPostParseMemory(def, parseFlags) < 0)
-return -1;
-
-if (virDomainDefAddConsoleCompat(def) < 0)
-return -1;
-
-if (virDomainDefRejectDuplicateControllers(def) < 0)
-return -1;
-
-if (virDomainDefRejectDuplicatePanics(def) < 0)
-return -1;
-
 /* verify settings of guest timers */
 for (i = 0; i < def->clock.ntimers; i++) {
 virDomainTimerDefPtr timer = def->clock.timers[i];
@@ -3845,6 +3824,36 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 }
 }
 
+return 0;
+}
+
+static int
+virDomainDefPostParseInternal(virDomainDefPtr def,
+  virCapsPtr caps ATTRIBUTE_UNUSED,
+  unsigned int parseFlags)
+{
+/* verify init path for container based domains */
+if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("init binary must be specified"));
+return -1;
+}
+
+if (virDomainDefPostParseMemory(def, parseFlags) < 0)
+return -1;
+
+if (virDomainDefAddConsoleCompat(def) < 0)
+return -1;
+
+if (virDomainDefRejectDuplicateControllers(def) < 0)
+return -1;
+
+if (virDomainDefRejectDuplicatePanics(def) < 0)
+return -1;
+
+if (virDomainDefPostParseTimer(def) < 0)
+return -1;
+
 /* clean up possibly duplicated metadata entries */
 virDomainDefMetadataSanitize(def);
 
-- 
2.5.0

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

[libvirt] [PATCH 06/12] qemu: Handle CanonicalizeMachine in post parse

[Cole Robinson]

Rather than open coding calls. I can't see any reason not to
---
 src/qemu/qemu_domain.c | 23 +++
 src/qemu/qemu_driver.c | 29 -
 2 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 26a29b1..48a2160 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1199,6 +1199,26 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
 
 
 static int
+qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
+{
+const char *canon;
+
+if (!(canon = virQEMUCapsGetCanonicalMachine(qemuCaps, def->os.machine)))
+return 0;
+
+if (STRNEQ(canon, def->os.machine)) {
+char *tmp;
+if (VIR_STRDUP(tmp, canon) < 0)
+return -1;
+VIR_FREE(def->os.machine);
+def->os.machine = tmp;
+}
+
+return 0;
+}
+
+
+static int
 qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
void *opaque)
@@ -1223,6 +1243,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0)
 goto cleanup;
 
+if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
+goto cleanup;
+
 ret = 0;
  cleanup:
 virObjectUnref(qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 51c6950..50ce514 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1684,26 +1684,6 @@ static int qemuConnectNumOfDomains(virConnectPtr conn)
 }
 
 
-static int
-qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
-{
-const char *canon;
-
-if (!(canon = virQEMUCapsGetCanonicalMachine(qemuCaps, def->os.machine)))
-return 0;
-
-if (STRNEQ(canon, def->os.machine)) {
-char *tmp;
-if (VIR_STRDUP(tmp, canon) < 0)
-return -1;
-VIR_FREE(def->os.machine);
-def->os.machine = tmp;
-}
-
-return 0;
-}
-
-
 static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
 const char *xml,
 unsigned int flags)
@@ -1749,9 +1729,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
-goto cleanup;
-
 if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
 goto cleanup;
 
@@ -7531,9 +7508,6 @@ static virDomainPtr 
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
-goto cleanup;
-
 if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
 goto cleanup;
 
@@ -15888,9 +15862,6 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr 
conn,
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
-goto cleanup;
-
 if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
 goto cleanup;
 
-- 
2.5.0

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

[libvirt] [PATCH 04/12] domain: Add virDomainDefAddImplicitDevices

[Cole Robinson]

It's just a combination of AddImplicitControllers, and AddConsoleCompat.
Every caller that wants ImplicitControllers also wants the ConsoleCompat
AFAICT, so lump them together. We also need it for future patches.
---
 src/conf/domain_conf.c   | 19 ++-
 src/conf/domain_conf.h   |  2 +-
 src/libvirt_private.syms |  2 +-
 src/qemu/qemu_command.c  |  2 +-
 src/qemu/qemu_driver.c   |  6 +++---
 src/vmx/vmx.c|  2 +-
 src/vz/vz_sdk.c  |  2 +-
 7 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5b9dab9..61dc650 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3842,9 +3842,6 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 if (virDomainDefPostParseMemory(def, parseFlags) < 0)
 return -1;
 
-if (virDomainDefAddConsoleCompat(def) < 0)
-return -1;
-
 if (virDomainDefRejectDuplicateControllers(def) < 0)
 return -1;
 
@@ -3854,7 +3851,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 if (virDomainDefPostParseTimer(def) < 0)
 return -1;
 
-if (virDomainDefAddImplicitControllers(def) < 0)
+if (virDomainDefAddImplicitDevices(def) < 0)
 return -1;
 
 /* clean up possibly duplicated metadata entries */
@@ -18216,7 +18213,7 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr 
def)
  * in the XML. This is for compat with existing apps which will
  * not know/care about  info in the XML
  */
-int
+static int
 virDomainDefAddImplicitControllers(virDomainDefPtr def)
 {
 if (virDomainDefAddDiskControllersForType(def,
@@ -18251,6 +18248,18 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
 return 0;
 }
 
+int
+virDomainDefAddImplicitDevices(virDomainDefPtr def)
+{
+if (virDomainDefAddConsoleCompat(def) < 0)
+return -1;
+
+if (virDomainDefAddImplicitControllers(def) < 0)
+return -1;
+
+return 0;
+}
+
 virDomainIOThreadIDDefPtr
 virDomainIOThreadIDFind(virDomainDefPtr def,
 unsigned int iothread_id)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ae6d546..4a91f24 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2718,7 +2718,7 @@ virDomainObjPtr virDomainObjParseFile(const char 
*filename,
 bool virDomainDefCheckABIStability(virDomainDefPtr src,
virDomainDefPtr dst);
 
-int virDomainDefAddImplicitControllers(virDomainDefPtr def);
+int virDomainDefAddImplicitDevices(virDomainDefPtr def);
 
 virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def,
   unsigned int iothread_id);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1e51dcf..0f2f66c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -197,7 +197,7 @@ virDomainControllerRemove;
 virDomainControllerTypeToString;
 virDomainCpuPlacementModeTypeFromString;
 virDomainCpuPlacementModeTypeToString;
-virDomainDefAddImplicitControllers;
+virDomainDefAddImplicitDevices;
 virDomainDefCheckABIStability;
 virDomainDefCheckDuplicateDiskInfo;
 virDomainDefCheckUnsupportedMemoryHotplug;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 66ca111..2ea87cc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -13862,7 +13862,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
 
 VIR_FREE(nics);
 
-if (virDomainDefAddImplicitControllers(def) < 0)
+if (virDomainDefAddImplicitDevices(def) < 0)
 goto error;
 
 if (virDomainDefPostParse(def, qemuCaps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1161aa0..51c6950 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8074,7 +8074,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
 dev->data.disk = NULL;
 if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
-if (virDomainDefAddImplicitControllers(vmdef) < 0)
+if (virDomainDefAddImplicitDevices(vmdef) < 0)
 return -1;
 if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
 return -1;
@@ -8099,7 +8099,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 if (virDomainHostdevInsert(vmdef, hostdev))
 return -1;
 dev->data.hostdev = NULL;
-if (virDomainDefAddImplicitControllers(vmdef) < 0)
+if (virDomainDefAddImplicitDevices(vmdef) < 0)
 return -1;
 if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
 return -1;
@@ -8141,7 +8141,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0)
 return -1;
 dev->data.chr = NULL;
-if (virDomainDefAddImplicitControllers(vmdef) < 0)
+if 

[libvirt] [PATCH 01/12] domain: separate out function for post parse console compat

[Cole Robinson]

This should be a no-op
---
 src/conf/domain_conf.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9d47846..ab22322 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3664,24 +3664,11 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
 return 0;
 }
 
-
 static int
-virDomainDefPostParseInternal(virDomainDefPtr def,
-  virCapsPtr caps ATTRIBUTE_UNUSED,
-  unsigned int parseFlags)
+virDomainDefAddConsoleCompat(virDomainDefPtr def)
 {
 size_t i;
 
-/* verify init path for container based domains */
-if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("init binary must be specified"));
-return -1;
-}
-
-if (virDomainDefPostParseMemory(def, parseFlags) < 0)
-return -1;
-
 /*
  * Some really crazy backcompat stuff for consoles
  *
@@ -3774,6 +3761,29 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 def->consoles[0]->targetType = 
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
 }
 
+return 0;
+}
+
+static int
+virDomainDefPostParseInternal(virDomainDefPtr def,
+  virCapsPtr caps ATTRIBUTE_UNUSED,
+  unsigned int parseFlags)
+{
+size_t i;
+
+/* verify init path for container based domains */
+if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("init binary must be specified"));
+return -1;
+}
+
+if (virDomainDefPostParseMemory(def, parseFlags) < 0)
+return -1;
+
+if (virDomainDefAddConsoleCompat(def) < 0)
+return -1;
+
 if (virDomainDefRejectDuplicateControllers(def) < 0)
 return -1;
 
-- 
2.5.0

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

[libvirt] [PATCH 03/12] domain: add implicit controllers from post parse

[Cole Robinson]

Seems like the natural fit, since we are already adding other XML bits
in the PostParse routine.

Previously AddImplicitControllers was only called at the end of XML
parsing, meaning code that builds a DomainDef by hand had to manually
call it. Adding it for those sites causes some test suite churn.
---
 src/conf/domain_conf.c   | 7 +++
 tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml   | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml   | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml   | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-sound.xml   | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml| 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml   | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv-v2.xml  | 1 +
 tests/sexpr2xmldata/sexpr2xml-fv.xml | 1 +
 tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml| 1 +
 tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml  | 1 +
 tests/xlconfigdata/test-fullvirt-multiusb.xml| 1 +
 tests/xlconfigdata/test-new-disk.xml | 1 +
 tests/xlconfigdata/test-spice-features.xml   | 1 +
 tests/xlconfigdata/test-spice.xml| 1 +
 tests/xmconfigdata/test-escape-paths.xml | 1 +
 tests/xmconfigdata/test-fullvirt-default-feature.xml | 1 +
 tests/xmconfigdata/test-fullvirt-force-hpet.xml  | 1 +
 tests/xmconfigdata/test-fullvirt-force-nohpet.xml| 1 +
 tests/xmconfigdata/test-fullvirt-localtime.xml   | 1 +
 tests/xmconfigdata/test-fullvirt-net-netfront.xml| 1 +
 tests/xmconfigdata/test-fullvirt-new-cdrom.xml   | 1 +
 tests/xmconfigdata/test-fullvirt-parallel-tcp.xml| 1 +
 tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml  | 1 +
 tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml | 1 +
 tests/xmconfigdata/test-fullvirt-serial-file.xml | 1 +
 tests/xmconfigdata/test-fullvirt-serial-null.xml | 1 +
 tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 1 +
 tests/xmconfigdata/test-fullvirt-serial-pty.xml  | 1 +
 tests/xmconfigdata/test-fullvirt-serial-stdio.xml| 1 +
 tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml   | 1 +
 tests/xmconfigdata/test-fullvirt-serial-tcp.xml  | 1 +
 tests/xmconfigdata/test-fullvirt-serial-udp.xml  | 1 +
 tests/xmconfigdata/test-fullvirt-serial-unix.xml | 1 +
 tests/xmconfigdata/test-fullvirt-sound.xml   | 1 +
 tests/xmconfigdata/test-fullvirt-usbmouse.xml| 1 +
 tests/xmconfigdata/test-fullvirt-usbtablet.xml   | 1 +
 tests/xmconfigdata/test-fullvirt-utc.xml | 1 +
 tests/xmconfigdata/test-no-source-cdrom.xml  | 1 +
 tests/xmconfigdata/test-pci-devs.xml | 1 +
 57 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2570f94..5b9dab9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3854,6 +3854,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 if (virDomainDefPostParseTimer(def) < 0)
 return -1;
 
+if (virDomainDefAddImplicitControllers(def) < 0)
+return -1;
+
 /* clean up possibly duplicated metadata entries */
 virDomainDefMetadataSanitize(def);
 
@@ -16396,10 +16399,6 @@ virDomainDefParseXML(xmlDocPtr xml,
 if (virDomainDefPostParse(def, caps, flags, xmlopt) < 0)
 goto error;
 
-/* Auto-add any implied controllers which aren't present */
-if (virDomainDefAddImplicitControllers(def) < 0)
-goto error;
-
 virHashFree(bootHash);
 
 return def;
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml 
b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml
index d68782d..17522f1 100644
--- 

[libvirt] [PATCH 00/12] Centralize more more bits in XML PostParse

[Cole Robinson]

There are several qemu and generic domain XML validating/populating
routines that callers need to invoke manually after parsing XML. This
series moves some of these calls into the PostParse handling routines.
Functionally there shouldn't be much change, except for more complete
XML in a few cases where drivers convert their native config formats.

Most of the patches are just moving functions around and verifying
things don't break, but there's some test suite improvements sprinkled
in.

The initial motivation for this series is to put qemuDomainAssignAddresses
into the PostParse call path, since I want that for future patches
tp autofill a user requested 

Cole Robinson (12):
  domain: separate out function for post parse console compat
  domain: separate out function for post parse timer validation
  domain: add implicit controllers from post parse
  domain: Add virDomainDefAddImplicitDevices
  qemu: domain: split out post parse default device handling
  qemu: Handle CanonicalizeMachine in post parse
  qemu: Handle SecurityManagerVerify in post parse
  domain: conf: Export virDomainDefPostParseDevices
  tests: qemuxml2xml: Use standard file comparison helpers
  tests: qemuxml2xml: Convert some fprintf to VIR_TEST_DEBUG
  tests: qemuxml2xml: Wire up QEMUCaps usage
  qemu: Assign device addresses in PostParse

 src/conf/domain_conf.c | 104 ++---
 src/conf/domain_conf.h |   6 +-
 src/libvirt_private.syms   |   3 +-
 src/qemu/qemu_command.c|   2 +-
 src/qemu/qemu_domain.c |  89 ++
 src/qemu/qemu_driver.c |  50 +-
 src/vmx/vmx.c  |   2 +-
 src/vz/vz_sdk.c|   2 +-
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |   4 +-
 tests/qemuxml2argvtest.c   |  12 +--
 .../qemuxml2xmlout-channel-virtio-auto.xml |   9 +-
 .../qemuxml2xmlout-disk-scsi-vscsi.xml |  35 +++
 .../qemuxml2xmlout-panic-pseries.xml   |  30 ++
 .../qemuxml2xmlout-pseries-panic-missing.xml   |   4 +-
 .../qemuxml2xmlout-pseries-panic-no-address.xml|   4 +-
 tests/qemuxml2xmltest.c|  39 +---
 tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml  |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml  |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml|   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml  |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml  |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml  |   1 +
 .../sexpr2xml-fv-serial-dev-2-ports.xml|   1 +
 .../sexpr2xml-fv-serial-dev-2nd-port.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml|   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml  |   1 +
 .../sexpr2xml-fv-serial-tcp-telnet.xml |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml|   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml|   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-sound.xml |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml  |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-utc.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-fv-v2.xml|   1 +
 tests/sexpr2xmldata/sexpr2xml-fv.xml   |   1 +
 tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml  |   1 +
 .../test-fullvirt-direct-kernel-boot.xml   |   1 +
 tests/xlconfigdata/test-fullvirt-multiusb.xml  |   1 +
 tests/xlconfigdata/test-new-disk.xml   |   1 +
 tests/xlconfigdata/test-spice-features.xml |   1 +
 tests/xlconfigdata/test-spice.xml  |   1 +
 tests/xmconfigdata/test-escape-paths.xml   |   1 +
 .../xmconfigdata/test-fullvirt-default-feature.xml |   1 +
 tests/xmconfigdata/test-fullvirt-force-hpet.xml|   1 +
 tests/xmconfigdata/test-fullvirt-force-nohpet.xml  |   1 +
 tests/xmconfigdata/test-fullvirt-localtime.xml |   1 +
 tests/xmconfigdata/test-fullvirt-net-netfront.xml  |   1 +
 tests/xmconfigdata/test-fullvirt-new-cdrom.xml |   1 +
 tests/xmconfigdata/test-fullvirt-parallel-tcp.xml  |   1 +
 .../test-fullvirt-serial-dev-2-ports.xml   |   1 +
 .../test-fullvirt-serial-dev-2nd-port.xml  |   1 +
 tests/xmconfigdata/test-fullvirt-serial-file.xml   |   1 +
 tests/xmconfigdata/test-fullvirt-serial-null.xml   |   1 +
 tests/xmconfigdata/test-fullvirt-serial-pipe.xml   

[libvirt] [PATCH 12/12] qemu: Assign device addresses in PostParse

[Cole Robinson]

In order to make this work, we need to short circuit the normal
virDomainDefPostParse ordering, and manually add stock devices
ourselves, since we need them in the XML before assigning addresses.

There's a bit of test suite churn due to extra XML output, and validation
happening at different call sites, but it all looks correct to me.

There's still quite a few manual callers of qemuDomainAssignAddresses
that could be dropped too but it would need additional testing.
---
 src/qemu/qemu_domain.c | 10 +++
 src/qemu/qemu_driver.c |  9 --
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  4 ++-
 tests/qemuxml2argvtest.c   | 12 ++--
 .../qemuxml2xmlout-channel-virtio-auto.xml |  9 +++---
 .../qemuxml2xmlout-disk-scsi-vscsi.xml | 35 ++
 .../qemuxml2xmlout-panic-pseries.xml   | 30 +++
 .../qemuxml2xmlout-pseries-panic-missing.xml   |  4 +--
 .../qemuxml2xmlout-pseries-panic-no-address.xml|  4 +--
 tests/qemuxml2xmltest.c| 10 +--
 10 files changed, 97 insertions(+), 30 deletions(-)
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a981310..e0520ab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1249,6 +1249,16 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 if (virSecurityManagerVerify(driver->securityManager, def) < 0)
 goto cleanup;
 
+/* Device defaults are normally set after calling the driver specific
+   PostParse routine (this function), but we need them earlier. */
+if (virDomainDefPostParseDevices(def, caps, driver->xmlopt) < 0)
+goto cleanup;
+if (virDomainDefAddImplicitDevices(def) < 0)
+goto cleanup;
+
+if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
+goto cleanup;
+
 ret = 0;
  cleanup:
 virObjectUnref(qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 459401a..4290c9e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1726,9 +1726,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
-goto cleanup;
-
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
@@ -7266,9 +7263,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr 
conn,
 if (qemuAssignDeviceAliases(def, qemuCaps) < 0)
 goto cleanup;
 
-if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
-goto cleanup;
-
 /* do fake auto-alloc of graphics ports, if such config is used */
 for (i = 0; i < def->ngraphics; ++i) {
 virDomainGraphicsDefPtr graphics = def->graphics[i];
@@ -7502,9 +7496,6 @@ static virDomainPtr 
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
-goto cleanup;
-
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
0, )))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
index 39f4a1f..256f8c6 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml
@@ -28,7 +28,9 @@
   
 
 
-
+
+  
+
 
 
 
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 63480ce..fb9630d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -304,14 +304,6 @@ static int testCompareXMLToArgvFiles(const char *xml,
 
 virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine);
 
-if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DEVICE)) {
-if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) {
-if (flags & FLAG_EXPECT_ERROR)
-goto ok;
-goto out;
-}
-}
-
 log = virtTestLogContentAndReset();
 VIR_FREE(log);
 virResetLastError();
@@ -1373,7 +1365,7 @@ mymain(void)
 QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION);
 DO_TEST("pseries-vio-user-assigned",
 QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
-DO_TEST_ERROR("pseries-vio-address-clash",
+DO_TEST_PARSE_ERROR("pseries-vio-address-clash",
 QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 

[libvirt] [PATCH 08/12] domain: conf: Export virDomainDefPostParseDevices

[Cole Robinson]

We will use this in upcoming patches
---
 src/conf/domain_conf.c   | 31 +--
 src/conf/domain_conf.h   |  4 
 src/libvirt_private.syms |  1 +
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 61dc650..52dd293 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4187,12 +4187,10 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 return virDomainDeviceDefPostParse(dev, data->def, data->caps, 
data->xmlopt);
 }
 
-
 int
-virDomainDefPostParse(virDomainDefPtr def,
-  virCapsPtr caps,
-  unsigned int parseFlags,
-  virDomainXMLOptionPtr xmlopt)
+virDomainDefPostParseDevices(virDomainDefPtr def,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt)
 {
 int ret;
 struct virDomainDefPostParseDeviceIteratorData data = {
@@ -4201,6 +4199,23 @@ virDomainDefPostParse(virDomainDefPtr def,
 .xmlopt = xmlopt,
 };
 
+if ((ret = virDomainDeviceInfoIterateInternal(def,
+  
virDomainDefPostParseDeviceIterator,
+  true,
+  )) < 0)
+return ret;
+
+return 0;
+}
+
+int
+virDomainDefPostParse(virDomainDefPtr def,
+  virCapsPtr caps,
+  unsigned int parseFlags,
+  virDomainXMLOptionPtr xmlopt)
+{
+int ret;
+
 /* call the domain config callback */
 if (xmlopt->config.domainPostParseCallback) {
 ret = xmlopt->config.domainPostParseCallback(def, caps,
@@ -4210,13 +4225,9 @@ virDomainDefPostParse(virDomainDefPtr def,
 }
 
 /* iterate the devices */
-if ((ret = virDomainDeviceInfoIterateInternal(def,
-  
virDomainDefPostParseDeviceIterator,
-  true,
-  )) < 0)
+if ((ret = virDomainDefPostParseDevices(def, caps, xmlopt)) < 0)
 return ret;
 
-
 if ((ret = virDomainDefPostParseInternal(def, caps, parseFlags)) < 0)
 return ret;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4a91f24..2bba554 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2497,6 +2497,10 @@ virDomainDefPostParse(virDomainDefPtr def,
   virCapsPtr caps,
   unsigned int parseFlags,
   virDomainXMLOptionPtr xmlopt);
+int
+virDomainDefPostParseDevices(virDomainDefPtr def,
+ virCapsPtr caps,
+ virDomainXMLOptionPtr xmlopt);
 
 static inline bool
 virDomainObjIsActive(virDomainObjPtr dom)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0f2f66c..58f2d22 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -230,6 +230,7 @@ virDomainDefParseFile;
 virDomainDefParseNode;
 virDomainDefParseString;
 virDomainDefPostParse;
+virDomainDefPostParseDevices;
 virDomainDefSetMemoryInitial;
 virDomainDefSetMemoryTotal;
 virDomainDefSetVcpus;
-- 
2.5.0

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

[libvirt] [PATCH 11/12] tests: qemuxml2xml: Wire up QEMUCaps usage

[Cole Robinson]

Future changes will make some of these tests dependent on specific
QEMUCaps flags, so wire up the basic handling.
---
 tests/qemuxml2xmltest.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index c0270d4..32c9fed 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -37,6 +37,8 @@ struct testInfo {
 
 char *outInactiveName;
 char *outInactiveFile;
+
+virQEMUCapsPtr qemuCaps;
 };
 
 static int
@@ -216,6 +218,8 @@ testInfoFree(struct testInfo *info)
 
 VIR_FREE(info->outInactiveName);
 VIR_FREE(info->outInactiveFile);
+
+virObjectUnref(info->qemuCaps);
 }
 
 
@@ -225,6 +229,13 @@ testInfoSet(struct testInfo *info,
 bool different,
 int when)
 {
+if (!(info->qemuCaps = virQEMUCapsNew()))
+goto error;
+
+if (qemuTestCapsCacheInsert(driver.qemuCapsCache, name,
+info->qemuCaps) < 0)
+goto error;
+
 if (virAsprintf(>inName, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml",
 abs_srcdir, name) < 0)
 goto error;
-- 
2.5.0

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

[libvirt] [PATCH 09/12] tests: qemuxml2xml: Use standard file comparison helpers

[Cole Robinson]

This also makes VIR_TEST_REGENERATE_OUTPUT work
---
 tests/qemuxml2xmltest.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index f967ceb..e3bd9c2 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -43,7 +43,7 @@ static int
 testXML2XMLHelper(const char *inxml,
   const char *inXmlData,
   const char *outxml,
-  const char *outXmlData,
+  const char *outXmlData ATTRIBUTE_UNUSED,
   bool live)
 {
 char *actual = NULL;
@@ -66,10 +66,8 @@ testXML2XMLHelper(const char *inxml,
 if (!(actual = virDomainDefFormat(def, format_flags)))
 goto fail;
 
-if (STRNEQ(outXmlData, actual)) {
-virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml);
+if (virtTestCompareToFile(actual, outxml) < 0)
 goto fail;
-}
 
 ret = 0;
 
-- 
2.5.0

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

[libvirt] [PATCH 07/12] qemu: Handle SecurityManagerVerify in post parse

[Cole Robinson]

Rather than open coding calls. I can't see any reason not to
---
 src/qemu/qemu_domain.c | 3 +++
 src/qemu/qemu_driver.c | 6 --
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 48a2160..a981310 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1246,6 +1246,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
 goto cleanup;
 
+if (virSecurityManagerVerify(driver->securityManager, def) < 0)
+goto cleanup;
+
 ret = 0;
  cleanup:
 virObjectUnref(qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 50ce514..459401a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1723,9 +1723,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 if (virDomainCreateXMLEnsureACL(conn, def) < 0)
 goto cleanup;
 
-if (virSecurityManagerVerify(driver->securityManager, def) < 0)
-goto cleanup;
-
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
@@ -7502,9 +7499,6 @@ static virDomainPtr 
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
 if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
 goto cleanup;
 
-if (virSecurityManagerVerify(driver->securityManager, def) < 0)
-goto cleanup;
-
 if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
def->emulator)))
 goto cleanup;
 
-- 
2.5.0

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

Re: [libvirt] [PATCH 2/2] libxl: support vif outgoing bandwidth QoS

[Jim Fehlig]

On 01/07/2016 07:48 AM, Michal Privoznik wrote:
> On 29.12.2015 02:09, Jim Fehlig wrote:
>> The libxl_device_nic structure supports specifying an outgoing rate
>> limit based on a time interval and bytes allowed per interval. In xl
>> config a rate limit is specified as "/s@". INTERVAL
>> is optional and defaults to 50ms.
>>
>> libvirt expresses outgoing limits by average (required), peak, burst,
>> and floor attributes in units of KB/s. This patch supports the outgoing
>> bandwidth limit by converting the average KB/s to bytes per interval
>> based on the same default interval (50ms) used by xl.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>  src/libxl/libxl_conf.c | 39 +++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 23c74e7..6320421 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -1093,6 +1093,7 @@ libxlMakeNic(virDomainDefPtr def,
>>  {
>>  bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
>>  virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
>> +virNetDevBandwidthPtr actual_bw;
>>  
>>  /* TODO: Where is mtu stored?
>>   *
>> @@ -1206,6 +1207,44 @@ libxlMakeNic(virDomainDefPtr def,
>>  #endif
>>  }
>>  
>> +/*
>> + * Set bandwidth.
>> + * From $xen-sources/docs/misc/xl-network-configuration.markdown:
>> + *
>> + *
>> + * Specifies the rate at which the outgoing traffic will be limited to.
>> + * The default if this keyword is not specified is unlimited.
>> + *
>> + * The rate may be specified as "/s" or optionally 
>> "/s@".
>> + *
>> + * `RATE` is in bytes and can accept suffixes:
>> + * GB, MB, KB, B for bytes.
>> + * Gb, Mb, Kb, b for bits.
>> + * `INTERVAL` is in microseconds and can accept suffixes: ms, us, s.
>> + * It determines the frequency at which the vif transmission credit
>> + * is replenished. The default is 50ms.
>> +
>> + * Vif rate limiting is credit-based. It means that for "1MB/s@20ms",
>> + * the available credit will be equivalent of the traffic you would have
>> + * done at "1MB/s" during 20ms. This will results in a credit of 20,000
>> + * bytes replenished every 20,000 us.
>> + *
>> + *
>> + * libvirt doesn't support the notion of rate limiting over an interval.
>> + * Similar to xl's behavior when interval is not specified, set a 
>> default
>> + * interval of 50ms and calculate the number of bytes per interval based
>> + * on the specified average bandwidth.
>> + */
>> +actual_bw = virDomainNetGetActualBandwidth(l_nic);
>> +if (actual_bw && actual_bw->out && actual_bw->out->average) {
>> +uint64_t bytes_per_sec = actual_bw->out->average * 1024;
>> +uint64_t bytes_per_interval =
>> +(((uint64_t) bytes_per_sec * 5UL) / 100UL);
>> +
>> +x_nic->rate_bytes_per_interval = bytes_per_interval;
>> +x_nic->rate_interval_usecs =  5UL;
>> +}
>> +
> Interesting. I'd expect:
>
> x_nic->rate_bytes_per_interval = bytes_per_sec;
> x_nic->rate_interval_usecs = 1000*1000;

For the most part I mimicked the Xen code and wanted to stick with the default
interval of 50ms, which has been the default for a long time. It is even
mentioned in some old RHEL5 docs

https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/5/html/Virtualization/sect-Virtualization-Tips_and_tricks-Limit_network_bandwidth_for_a_Xen_guest.html

BTW, here is the Xen code that inspired this logic

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxlu_vif.c;h=0665e624dc178a6ca8058e04a7baacaf1475bd37;hb=HEAD#l131

rate_bytes_per_interval is set to (bytes/s * interval us)/100us

I guess we are saying the same thing, you're just setting interval to 1s (thus
rate_bytes_per_interval == bytes_per_sec) instead of the historical 50ms :-).

>
> I mean, if I understood the xl way of rate limiting correctly, one says
> how much bytes can be sent for how long. so for 1MB/s I'd expect to send
> 1024*1024 bytes each second.
>
> Or am I missing something?

Does the above explanation make sense? I might be missing something :-).  CC'd a
few Xen tools maintainer just in case.

Regards,
Jim

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

Re: [libvirt] [PATCH] pci: Log debug messages when manipulating the inactive list

[Peter Krempa]

On Thu, Jan 07, 2016 at 18:21:18 +0100, Andrea Bolognani wrote:
> Most of the changes to the list of active and inactive PCI devices
> happen in virHostdev, where they are properly logged.
> 
> virPCIDeviceDetach() and virPCIDeviceReattach(), however, change the
> inactive list as well, so they should be logging similar messages.
> ---
>  src/util/virpci.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

ACK


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

Re: [libvirt] [PATCH 09/12] tests: qemuxml2xml: Use standard file comparison helpers

[Peter Krempa]

On Thu, Jan 07, 2016 at 22:50:03 -0500, Cole Robinson wrote:
> This also makes VIR_TEST_REGENERATE_OUTPUT work
> ---
>  tests/qemuxml2xmltest.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index f967ceb..e3bd9c2 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -43,7 +43,7 @@ static int
>  testXML2XMLHelper(const char *inxml,
>const char *inXmlData,
>const char *outxml,
> -  const char *outXmlData,
> +  const char *outXmlData ATTRIBUTE_UNUSED,

Since there are just two callers of this you might as well as remove
this argument rather than making it unused.



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

Re: [libvirt] [PATCH 1/2] xenconfig: support parsing and formatting vif bandwidth

[Jim Fehlig]

On 01/07/2016 07:48 AM, Michal Privoznik wrote:
> On 29.12.2015 02:09, Jim Fehlig wrote:
>> Both xm and xl config have long supported specifying vif rate
>> limiting, e.g.
>>
>> vif = [ 'mac=00:16:3E:74:3d:76,bridge=br0,rate=10MB/s' ]
>>
>> Add support for mapping rate to and from  in the xenconfig
>> parser and formatter. rate is mapped to the required 'average' attribute
>> of the  element, e.g.
>>
>>   
>> ...
>> 
>>   
>> 
>>   
>>
>> Also add a unit test to check the conversion logic.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>
>> I used a bit of code from libxlu_vif.c to implement xenParseVifRate()
>> instead of using the libxlutil lib directly, since in theory rate limiting
>> applies to the old xen driver (no libxl) as well.
>>
>>  src/xenconfig/xen_common.c   | 77 
>> 
>>  tests/xlconfigdata/test-vif-rate.cfg | 26 
>>  tests/xlconfigdata/test-vif-rate.xml | 57 ++
>>  tests/xlconfigtest.c |  1 +
>>  4 files changed, 161 insertions(+)
> ACK

Hi Michal,

Thanks a lot for taking a look at this series! Note that I posted a V2 earlier
this week which includes support for parsing/formatting vif bandwidth in sexpr
config too

https://www.redhat.com/archives/libvir-list/2016-January/msg00045.html

Unfortunately it results in some changes to this patch, so I think it would be
best to peek at the V2 series before pushing it.

Regards,
Jim

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

Re: [libvirt] [PATCH 01/12] domain: separate out function for post parse console compat

[Peter Krempa]

On Thu, Jan 07, 2016 at 22:49:55 -0500, Cole Robinson wrote:
> This should be a no-op
> ---
>  src/conf/domain_conf.c | 38 --
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9d47846..ab22322 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3664,24 +3664,11 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
>  return 0;
>  }
>  
> -

This file uses two newlines to separate functions ..

>  static int
> -virDomainDefPostParseInternal(virDomainDefPtr def,
> -  virCapsPtr caps ATTRIBUTE_UNUSED,
> -  unsigned int parseFlags)
> +virDomainDefAddConsoleCompat(virDomainDefPtr def)
>  {
>  size_t i;
>  
> -/* verify init path for container based domains */
> -if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
> -virReportError(VIR_ERR_XML_ERROR, "%s",
> -   _("init binary must be specified"));
> -return -1;
> -}
> -
> -if (virDomainDefPostParseMemory(def, parseFlags) < 0)
> -return -1;
> -
>  /*
>   * Some really crazy backcompat stuff for consoles
>   *
> @@ -3774,6 +3761,29 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
>  def->consoles[0]->targetType = 
> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
>  }
>  
> +return 0;
> +}

... here too ...

> +
> +static int
> +virDomainDefPostParseInternal(virDomainDefPtr def,
> +  virCapsPtr caps ATTRIBUTE_UNUSED,
> +  unsigned int parseFlags)
> +{
> +size_t i;
> +
> +/* verify init path for container based domains */
> +if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("init binary must be specified"));
> +return -1;
> +}
> +
> +if (virDomainDefPostParseMemory(def, parseFlags) < 0)
> +return -1;
> +
> +if (virDomainDefAddConsoleCompat(def) < 0)
> +return -1;
> +
>  if (virDomainDefRejectDuplicateControllers(def) < 0)
>  return -1;

ACK with whitespace fixed.


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

Re: [libvirt] [PATCH 02/12] domain: separate out function for post parse timer validation

[Peter Krempa]

On Thu, Jan 07, 2016 at 22:49:56 -0500, Cole Robinson wrote:
> This should be a no-op
> ---
>  src/conf/domain_conf.c | 53 
> +-
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ab22322..2570f94 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -3845,6 +3824,36 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
>  }
>  }
>  
> +return 0;
> +}

Two newlines please

> +
> +static int
> +virDomainDefPostParseInternal(virDomainDefPtr def,
> +  virCapsPtr caps ATTRIBUTE_UNUSED,
> +  unsigned int parseFlags)
> +{
> +/* verify init path for container based domains */
> +if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("init binary must be specified"));
> +return -1;

ACK


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

Re: [libvirt] [PATCH 0/3] several cgroups/cpuset fixes

[John Ferlan]

[...]

>> No problem - although it seems they've generated a regression in the
>> virttest memtune test suite.  I'm 'technically' on vacation for the
>> next couple of weeks; however, I think/perhaps the problem is a
>> result of this patch and the change to adding the task to the cgroup
>> at the end of the for loop, but perhaps the following code causes the
>> control to jump back to the top of the loop:
>>
>>  if (!cpumap)
>>  continue;
>>
>>   if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
>>  goto cleanup;
>>
>> not allowing the
>>
>>
>> /* move the thread for vcpu to sub dir */
>> if (virCgroupAddTask(cgroup_vcpu,
>>  qemuDomainGetVcpuPid(vm, i)) < 0)
>> goto cleanup;
>>
>> to be executed.
>>
>> The code should probably change to be (like IOThreads):
>>
>>  if (cpumap &&
>>  qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
>>  goto cleanup;
>>
>>
>> As for the rest, I suspect things will be quite quiet around here over
>> the next couple of weeks. A discussion to perhaps start in the new
>> year.
> 
> Same here. I will have a look at that regression after my vacation,
> should it still be there.
> 
> Henning
> 

More data from the issue...  While the above mentioned path is an issue,
I don't believe it's what's causing the test failure.

I haven't quite figured out why yet, but it seems the /proc/#/cgroup
file isn't getting the proper path for the 'memory' slice and thus the
test fails because it's looking at the:

   /sys/fs/cgroup/memory/machine.slice/memory.*

files instead of the

/sys/fs/cgroup/memory/machine.slice/$path/memory.*

Where $path is "machine-qemu\x2dvirt\x2dtests\x2dvm1.scope"

This affects the virsh memtune $dom command test suite which uses the
/proc/$pid/cgroup file in order to find the path for the 'memory' or
'cpuset' or 'cpu,cpuacct' cgroup paths.

Seems to be some interaction with systemd that I have quite figured out.

I'm assuming this is essentially the issue you were trying to fix - that
is changes to values should be done to the machine-qemu* specific files
rather than the machine.slice files.

The good news is I can see the changes occurring in the machine-qemu*
specific files, so it seems libvirt is doing the right thing.

However, there's something strange with perhaps previously
existing/running domains where that /proc/$pid/cgroup file doesn't get
the $path for the memory entry, thus causing the test validation to look
in the wrong place.

Hopefully this makes sense. What's really strange (for me at least) is
that it's only occurring on one test system. I can set up the same test
on another system and things work just fine.  I'm not quite sure what
interaction generates that /proc/$pid/cgroup file - hopefully someone
else understands it and help me make sense of it.

John









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

[libvirt] looking again at add nested HVM to libxl

[Alvin Starr]

I have been looking at this once again and thinking about this whole 
implementation.


I can agree with Daniel's assesment of the SVM/NPT bit setting but the 
nested_hvm feature does not exist at all in  KVM at the libvirt level/
I could be wrong but when I last looked nested hvm could only be 
enabled/disabled in KVM at boot time and not on a per VM basis.


What is the accepted policy on features that are only available in a 
single VM engine?


I could remove the SVM/NPT bit twidling and resbumit the patch  if it is 
likely to go somewhere?


On 10/02/2015 02:44 PM, Alvin Starr wrote:

I agree that it needs to be done the right way.

I am willing to help where I can but my basic knowledge of the code is 
badly lacking so there is no reasonable way for me to contribute in a 
significant way.


I am ok keeping my own hacks to the code that let me work along on my 
projects and hope that some day this will get fixed.



On 10/02/2015 10:03 AM, Daniel P. Berrange wrote:

On Fri, Oct 02, 2015 at 09:50:26AM -0400, Alvin Starr wrote:

I wondered about this.

I first looked at using the CPU feature bit manipulation but that 
was, at

the time, completely unimplemented.
The CPU feature code also looked to be somewhat KVM centric.

In the end I went for the simple implementation because the 
complexity of

implementing the feature code would mean that I would need to become a
libvirt developer and I mostly just want to be a helpful libvirt user.

Yep, I can certainly understand that POV. Unfortunately I think in this
case we really do need to do it the right way. IIUC, Xen does indeed 
allow

the CPUID mask to be configured, so it ought to be possible to map from
the libvirt XML to the CPUID mask format. It would certainly be a 
non-negligble

bit of work though.


Regards,
Daniel






--
Alvin Starr   ||   voice: (905)513-7688
Netvel Inc.   ||   Cell:  (416)806-0133
al...@netvel.net  ||

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

Re: [libvirt] [PATCH 0/3] several cgroups/cpuset fixes

[Henning Schild]

On Thu, 7 Jan 2016 11:20:23 -0500
John Ferlan  wrote:

> 
> [...]
> 
> >> No problem - although it seems they've generated a regression in
> >> the virttest memtune test suite.  I'm 'technically' on vacation
> >> for the next couple of weeks; however, I think/perhaps the problem
> >> is a result of this patch and the change to adding the task to the
> >> cgroup at the end of the for loop, but perhaps the following code
> >> causes the control to jump back to the top of the loop:
> >>
> >>  if (!cpumap)
> >>  continue;
> >>
> >>   if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) <
> >> 0) goto cleanup;
> >>
> >> not allowing the
> >>
> >>
> >> /* move the thread for vcpu to sub dir */
> >> if (virCgroupAddTask(cgroup_vcpu,
> >>  qemuDomainGetVcpuPid(vm, i)) < 0)
> >> goto cleanup;
> >>
> >> to be executed.
> >>
> >> The code should probably change to be (like IOThreads):
> >>
> >>  if (cpumap &&
> >>  qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) <
> >> 0) goto cleanup;
> >>
> >>
> >> As for the rest, I suspect things will be quite quiet around here
> >> over the next couple of weeks. A discussion to perhaps start in
> >> the new year.
> > 
> > Same here. I will have a look at that regression after my vacation,
> > should it still be there.
> > 
> > Henning
> > 
> 
> More data from the issue...  While the above mentioned path is an
> issue, I don't believe it's what's causing the test failure.
>
> I haven't quite figured out why yet, but it seems the /proc/#/cgroup
> file isn't getting the proper path for the 'memory' slice and thus the
> test fails because it's looking at the:
> 
>/sys/fs/cgroup/memory/machine.slice/memory.*
> 
> files instead of the
> 
> /sys/fs/cgroup/memory/machine.slice/$path/memory.*

To be honest i did just look at the cgroup/cpuset/ hierarchy, but i
just browsed cgroup/memory/ as well.

The target of my patch series was to get
cgroup/cpuset/machine.slice/tasks to be emtpy, all tasks should be in
their sub-cgroup under the machine.slice. And the ordering patches make
sure the file is always empty.

In the memory cgroups all tasks are in the parent group (all in
machine.slice/tasks). machine.slice/*/tasks are empty. I am not sure
whether that is intended, i can just assume it is a bug in the memory
cgroup subsystem. Why are the groups created and tuned when the tasks
stay in the big superset?
/proc/#/cgroup is showing the correct path, libvirt seems to fail to
migrate tasks into memory subgroups. (i am talking about a patched
1.2.19 where vms do not have any special memory tuning)

Without my patches the first qemu thread was in
"2:cpuset:/machine.slice" and the name did match
"4:memory:/machine.slice". Now if the test wants matching names the
test might just be wrong. Or as indicated before there might be a bug
in the memory cgroups.

> Where $path is "machine-qemu\x2dvirt\x2dtests\x2dvm1.scope"
> 
> This affects the virsh memtune $dom command test suite which uses the
> /proc/$pid/cgroup file in order to find the path for the 'memory' or
> 'cpuset' or 'cpu,cpuacct' cgroup paths.
> 
> Seems to be some interaction with systemd that I have quite figured
> out.
> 
> I'm assuming this is essentially the issue you were trying to fix -
> that is changes to values should be done to the machine-qemu*
> specific files rather than the machine.slice files.
> 
> The good news is I can see the changes occurring in the machine-qemu*
> specific files, so it seems libvirt is doing the right thing.
> 
> However, there's something strange with perhaps previously
> existing/running domains where that /proc/$pid/cgroup file doesn't get
> the $path for the memory entry, thus causing the test validation to
> look in the wrong place.
> 
> Hopefully this makes sense. What's really strange (for me at least) is
> that it's only occurring on one test system. I can set up the same
> test on another system and things work just fine.  I'm not quite sure
> what interaction generates that /proc/$pid/cgroup file - hopefully
> someone else understands it and help me make sense of it.

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

Re: [libvirt] Libvirt 1.3.0 compilation failure on OSX 10.9.5

[Justin Clift]

On 6 Jan 2016, at 18:03, Roman Bogorodskiy  wrote:
>  Roman Bogorodskiy wrote:
>>  Justin Clift wrote:
>>> Hiyas,
>>> 
>>> Just went to update the MacOS X Homebrew port/formula for
>>> Libvirt 1.3.0.  It's failing during compilation with:
>>> 
>>> ***
>>> 
>>>  Last 15 lines from /Users/jc/Library/Logs/Homebrew/libvirt/02.make:
>>>CCLD libvirt_driver_vbox.la
>>>CCLD libvirt_driver_storage.la
>>>  Undefined symbols for architecture x86_64:
>>>"_xdr_uint64_t", referenced from:
>>>_xdr_virLogManagerProtocolLogFilePosition in virtlogd-log_protocol.o
>>>_xdr_virLogManagerProtocolDomainOpenLogFileRet in 
>>> virtlogd-log_protocol.o
>>>_xdr_virLogManagerProtocolDomainGetLogFilePositionRet in 
>>> virtlogd-log_protocol.o
>>>_xdr_virLogManagerProtocolDomainReadLogFileArgs in 
>>> virtlogd-log_protocol.o
>>>  ld: symbol(s) not found for architecture x86_64
>>>  clang: error: linker command failed with exit code 1 (use -v to see 
>>> invocation)
>>>  make[3]: *** [virtlogd] Error 1
>>>  make[3]: *** Waiting for unfinished jobs
>>>  make[2]: *** [all] Error 2
>>>  make[1]: *** [all-recursive] Error 1
>>>  make: *** [all] Error 2
>>> 
>>> ***
>>> 
>>> Is this a known thing, or should I grab the rest of the failure logging for
>>> someone (not me) to look at? :)
>>> 
>> Hi Justin,
>> 
>> Could you check if this patche solves the problem for you:
>> 
>> http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/sysutils/libvirt/patches/patch-src_admin_admin_protocol_c?rev=1.2=text/x-cvsweb-markup
>> ?
> 
> Urgh, that's the wrong one. The right one is here:
> 
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/sysutils/libvirt/patches/patch-src_logging_log_protocol_c?rev=1.2=text/x-cvsweb-markup
> 
> But as they're identical, I'd assume you'll need the first one as well.

Interestingly, with just this one added in, libvirt now compiles fully and
installs fine.  Didn't need to touch admin_protocol.c at all.  Maybe that's
only conditionally compiled or something?

virsh seems to start up ok, but I didn't actually try doing anything with
it. ;)

Should we call this "good enough for now", or is there more to do? :)

+ Justin

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

Re: [libvirt] [PATCH] qemu: fix default migration_address for NBD server

[Jiri Denemark]

On Thu, Dec 31, 2015 at 16:51:26 +1100, Michael Chapman wrote:
> If no migration_address is configured, QEMU will listen on 0.0.0.0 or
> [::].  Commit 674afcb09e3d33500cfbbcf870ebf92cb99ecfa3 moved the
> handling of this default value into a new qemuMigrationPrepareIncoming
> function, however the address is also needed when starting the NBD
> server from within qemuMigrationPrepareAny.

Thanks for noticing and analyzing the issue, however your patch
basically reverts the refactoring, which is not the best way to fix this
issue. I'll take care of this...

Jirka

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

Re: [libvirt] [PATCH 2/4] virsh: Add timestamps to events

[Jiri Denemark]

On Mon, Dec 21, 2015 at 16:25:56 +0100, Andrea Bolognani wrote:
> On Mon, 2015-12-21 at 11:04 +0100, Jiri Denemark wrote:
> > 
> > @@ -12154,7 +12155,16 @@ virshEventPrint(virshDomEventData *data,
> >  if (!data->loop && *data->count)
> >  goto cleanup;
> >  
> > -vshPrint(data->ctl, "%s", msg);
> > +if (data->timestamp) {
> > +char timestamp[VIR_TIME_STRING_BUFLEN];
> > +
> > +if (virTimeStringNowRaw(timestamp) < 0)
> > +timestamp[0] = '\0';
> > +
> > +vshPrint(data->ctl, "%s: %s", timestamp, msg);
> > +} else {
> > +vshPrint(data->ctl, "%s", msg);
> > +}
> 
> So the timestamp is not really for the moment the even occurred,
> rather the moment it was printed. I don't know if the difference is
> something we should really care about.

The main reason for adding the timestamp was to be able to look at the
output and see, e.g., that a few events came very close to each other,
then there was a 30s delay before another event came. I don't think
getting the exact timing from libvirtd would be useful for anything.

> The signature for virConnectDomainEventGenericCallback, unlike the one
> for virConnectDomainQemuMonitorEventCallback, does not provide timining
> information. Would it be a good idea to maybe have a new kind of
> callback for generic events that includes such information, instead of
> using the timestamp from when we printed the message?

It's not just about the callback signature, each event would have to
provide the timestamp... IMHO it's not worth the effort at all.

> What about having qemu-monitor-event support --timestamp as well? I know
> the timing information are already part of the output, but we could use
> the same format used here (much more readable) and avoid duplicating
> them in the message. POC attached.

qemu-monitor-event is designed to provide more-or-less raw data from
QEMU. Personally, I'd leave it alone, but I don't feel strongly either
way :-) The main reason for doing it this way was laziness (what a
surprise!).

Jirka

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

Re: [libvirt] [PATCH 1/4] virsh: Refactor event printing

[Jiri Denemark]

On Mon, Dec 21, 2015 at 14:46:33 +0100, Andrea Bolognani wrote:
> n Mon, 2015-12-21 at 11:04 +0100, Jiri Denemark wrote:
> > To reduce code duplication.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  tools/virsh-domain.c | 293 
> >---
> >  1 file changed, 136 insertions(+), 157 deletions(-)
> 
> Really nice cleanup, looks good overall.
> 
> A couple of comments below.
> 
> >  static void
> > -virshEventPrint(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom,
> > -const char *event, long long seconds, unsigned int micros,
> > -const char *details, void *opaque)
> > +virshEventQemuPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
> > +virDomainPtr dom,
> > +const char *event,
> > +long long seconds,
> > +unsigned int micros,
> > +const char *details,
> > +void *opaque)
> >  {
> >  virshQemuEventData *data = opaque;
> >  virJSONValuePtr pretty = NULL;
> 
> A comment describing the functions would be nice. Their use is
> pretty obvious, so not a deal breaker.

Done. Is it a deal? :-)

> >  static void
> > +virshEventPrint(virshDomEventData *data,
> > +virBufferPtr buf)
> > +{
> > +char *msg;
> > +
> > +if (!(msg = virBufferContentAndReset(buf)))
> > +return;
> > +
> > +if (!data->loop && *data->count)
> > +goto cleanup;
> > +
> > +vshPrint(data->ctl, "%s", msg);
> > +
> > +(*data->count)++;
> > +if (!data->loop)
> > +vshEventDone(data->ctl);
> > +
> > + cleanup:
> > +VIR_FREE(msg);
> > +}
> 
> This works just fine, however I dislike the fact that the virBuffer
> is allocated by the caller but released here.
> 
> I'd rather use virBufferCurrentContent() here and leave the caller
> responsible for releasing the buffer. Doing so would make the callers
> slightly more verbose, but result in cleaner code overall IMHO.

I think we just need a comment explicitly saying that this function will
free the buffer. And I added it to the description requested by your
not-a-deal-breaker comment.

Is this good enough?

+/**
+ * virshEventPrint:
+ *
+ * @data: opaque data passed to all event callbacks
+ * @buf: string buffer describing the event
+ *
+ * Print the event description found in @buf and update virshDomEventData.
+ *
+ * This function resets @buf and frees all memory consumed by its content.
+ */

> > @@ -12237,26 +12249,26 @@ virshEventGraphicsPrint(virConnectPtr conn 
> > ATTRIBUTE_UNUSED,
> >  const virDomainEventGraphicsSubject *subject,
> >  void *opaque)
> >  {
> > -virshDomEventData *data = opaque;
> > +virBuffer buf = VIR_BUFFER_INITIALIZER;
> >  size_t i;
> >  
> > -if (!data->loop && *data->count)
> > -return;
> > -vshPrint(data->ctl, _("event 'graphics' for domain %s: "
> > -  "%s local[%s %s %s] remote[%s %s %s] %s"),
> > - virDomainGetName(dom), virshGraphicsPhaseToString(phase),
> > - virshGraphicsAddressToString(local->family),
> > - local->node, local->service,
> > - virshGraphicsAddressToString(remote->family),
> > - remote->node, remote->service,
> > - authScheme);
> > -for (i = 0; i < subject->nidentity; i++)
> > -vshPrint(data->ctl, " %s=%s", subject->identities[i].type,
> > - subject->identities[i].name);
> > -vshPrint(data->ctl, "\n");
> > -(*data->count)++;
> > -if (!data->loop)
> > -vshEventDone(data->ctl);
> > +virBufferAsprintf(, _("event 'graphics' for domain %s: "
> > +  "%s local[%s %s %s] remote[%s %s %s] %s\n"),
> > +  virDomainGetName(dom),
> > +  virshGraphicsPhaseToString(phase),
> > +  virshGraphicsAddressToString(local->family),
> > +  local->node,
> > +  local->service,
> > +  virshGraphicsAddressToString(remote->family),
> > +  remote->node,
> > +  remote->service,
> > +  authScheme);
> > +for (i = 0; i < subject->nidentity; i++) {
> > +virBufferAsprintf(, "\t%s=%s\n",
> > +  subject->identities[i].type,
> > +  subject->identities[i].name);
> > +}
> > +virshEventPrint(opaque, );
> >  }
> 
> You're changing the format a bit here - I like the new one better, and
> it's the same used in virshEventTunablePrint() below. I'm just concerned
> about users parsing this in some way and being confused by the change.

Right, I changed the format to be consistent with another event which
also reported name=value stuff. I don't think we need to be worried
about the