Re: [libvirt] [PATCH v3 00/18] Implement virDomainBlockCopy

[Jiri Denemark]

On Sat, Aug 30, 2014 at 22:02:18 -0600, Eric Blake wrote:
> Took me longer than I wanted to get v3 posted.  There's lots of
> new patches in this version, based on feedback on v2.
> 
> Among other things, the bandwidth of virDomainBlockCopy is in
> bytes/s, and all the remaining interfaces are updated to use a
> flags value to decide whether to use bytes/s instead of the
> back-compat MiB/s.
> 
> I really want patch 1 in 1.2.8; pasth 2-17 would be nice but
> it may be better to delay to later, and I still don't have
> virsh updated to cover the changed proposed in patch 18 so that
> one should probably wait.

Patch 1 is easy and safe to be included in 1.2.8 but I think it's way
too late for the rest. However, since the implementation will be missing
from 1.2.8, I think the best solution would be to just revert the patch
adding this public API and let it slip to the next release.

Jirka

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

Re: [libvirt] [PATCH] virsh: Implement command to rename domain

[Philipp Hahn]

Hello Tomas,

On 01.09.2014 01:51, Tomas Meszaros wrote:
> I've recently worked with rather large number of virtual machines
> and needed to rename all domains. I couldn't find better way how
> to rename domain other than:
> 
> virsh dumpxml domain > domain.xml
> (change domain name in domain.xml)
> virsh undefine domain
> virsh define domain.xml
> 
> This is rather pain to do every time I want to rename domain.
> I think there should be simple way to change domain name.

This has been requested in the past already (even by me ;-)
Renaming is not that simple, as there are several more things to do:
1. Rename log files (this was somehow controversial last time it was
discussed, especially combined with external programs like logrotate)
2. Fix domain config for suspended VMs.
3. Keep existing snapshots
3.1 Fix domain config in snapshots.

Especially the last thing does very bad things if you revert a renamed
VM, as the UUID is then no longer unique.

Philipp "sorry, no patch" Hahn

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

Re: [libvirt] [PATCH] Fix connection to already running session libvirtd

[Christophe Fergeau]

Hi,

On Fri, Aug 29, 2014 at 10:22:33AM -0600, Eric Blake wrote:
> Sometimes, when a patch is that invasive, I'll do it in two parts - the
> change with wrong indentation, followed by another patch that is
> indentation-only.  Much easier to review.

Ah right, I remember seeing that in the past, I didn't think at all
about doing that for this patch :( With 1.2.8 being so close, I'll
push v2 though instead of doing this in a v3 as I'd rather not miss the
release because of the additional round of review.

> 
> Also, are you using the patience algorithm with git? (git config
> diff.algorithm patience)  It makes indentation patches easier to review,
> by not trying to interleave lone } and blank lines that correspond to
> different code from pre- and post-patch.

I've changed that now, and the diff is indeed much better, thanks for
the tip!

Christophe


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

Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group

[Francesco Romani]

- Original Message -
> From: "Li Wei" 
> To: "Francesco Romani" , libvir-list@redhat.com
> Sent: Monday, September 1, 2014 7:32:37 AM
> Subject: Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group
> 
> Hi Francesco,
> 
> I notice your patchset is much complete than mine which only focus on
> VIR_DOMAIN_STATS_BLOCK[1], but it seems your patch implement block stats
> query in a per-block style, this should be a bottleneck when there are
> a lot of block devices in a domain.
> 
> Could you implement it in a bulk style? so we just need only one qmp-command
> for each domain.
> 
> [1]: https://www.redhat.com/archives/libvir-list/2014-August/msg01497.html


Hi Li Wei,

Thanks for pointing this out. Performance is surely a major concern of mine,
then I'll improve my patch in the direction you outlined.

I read your patch (actually I like some parts of your patch more than mine!)
but I must somehow missed that.

I'll wait for more reviews before to resubmit.

Thanks and bests,

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani

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

Re: [libvirt] [Qemu-devel] IO accounting overhaul

[Markus Armbruster]

Cc'ing libvirt following Stefan's lead.

Benoît Canet  writes:

> Hi,
>
> I collected some items of a cloud provider wishlist regarding I/O accouting.

Feedback from real power-users, lovely!

> In a cloud I/O accouting can have 3 purpose: billing, helping the customers
> and doing metrology to help the cloud provider seeks hidden costs.
>
> I'll cover the two former topic in this mail because they are the most 
> important
> business wize.
>
> 1) prefered place to collect billing IO accounting data:
> 
> For billing purpose the collected data must be as close as possible to what 
> the
> customer would see by using iostats in his vm.

Good point.

> The first conclusion we can draw is that the choice of collecting IO accouting
> data used for billing in the block devices models is right.

Slightly rephrasing: doing I/O accounting in the block device models is
right for billing.

There may be other uses for I/O accounting, with different preferences.
For instance, data on how exactly guest I/O gets translated to host I/O
as it flows through the nodes in the block graph could be useful.

Doesn't diminish the need for accurate billing information, of course.

> 2) what to do with occurences of rare events:
> -
>
> Another point is that QEMU developpers agree that they don't know which policy
> to apply to some I/O accounting events.
> Must QEMU discard invalid I/O write IO or account them as done ?
> Must QEMU count a failed read I/O as done ?
>
> When discusting this with a cloud provider the following appears:
> these decisions
> are really specific to each cloud provider and QEMU should not implement them.

Good point, consistent with the old advice to avoid baking policy into
inappropriately low levels of the stack.

> The right thing to do is to add accouting counters to collect these events.
>
> Moreover these rare events are precious troubleshooting data so it's
> an additional
> reason not to toss them.

Another good point.

> 3) list of block I/O accouting metrics wished for billing and helping
> the customers
> ---
>
> Basic I/O accouting data will end up making the customers bills.
> Extra I/O accouting informations would be a precious help for the cloud 
> provider
> to implement a monitoring panel like Amazon Cloudwatch.

These are the first two from your list of three purposes, i.e. the ones
you promised to cover here.

> Here is the list of counters and statitics I would like to help
> implement in QEMU.
>
> This is the most important part of the mail and the one I would like
> the community
> review the most.
>
> Once this list is settled I would proceed to implement the required
> infrastructure
> in QEMU before using it in the device models.

For context, let me recap how I/O accounting works now.

The BlockDriverState abstract data type (short: BDS) can hold the
following accounting data:

uint64_t nr_bytes[BDRV_MAX_IOTYPE];
uint64_t nr_ops[BDRV_MAX_IOTYPE];
uint64_t total_time_ns[BDRV_MAX_IOTYPE];
uint64_t wr_highest_sector;

where BDRV_MAX_IOTYPE enumerates read, write, flush.

wr_highest_sector is a high watermark updated by the block layer as it
writes sectors.

The other three are *not* touched by the block layer.  Instead, the
block layer provides a pair of functions for device models to update
them:

void bdrv_acct_start(BlockDriverState *bs, BlockAcctCookie *cookie,
int64_t bytes, enum BlockAcctType type);
void bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie);

bdrv_acct_start() initializes cookie for a read, write, or flush
operation of a certain size.  The size of a flush is always zero.

bdrv_acct_done() adds the operations to the BDS's accounting data.
total_time_ns is incremented by the time between _start() and _done().

You may call _start() without calling _done().  That's a feature.
Device models use it to avoid accounting some requests.

Device models are not supposed to mess with cookie directly, only
through these two functions.

Some device models implement accounting, some don't.  The ones that do
don't agree on how to count invalid guest requests (the ones not passed
to block layer) and failed requests (passed to block layer and failed
there).  It's a mess in part caused by us never writing down what
exactly device models are expected to do.

Accounting data is used by "query-blockstats", and nothing else.

Corollary: even though every BDS holds accounting data, only the ones in
"top" BDSes ever get used.  This is a common block layer blemish, and
we're working on cleaning it up.

If a device model doesn't implement accounting, query-blockstats lies.
Fortunately, its lies are pretty transparent (everything's zero) as long
as you don't do things like connecting a backend to a device model that
doesn't implement accounting after disconnecting i

Re: [libvirt] [Qemu-devel] IO accounting overhaul

[Benoît Canet]

The Monday 01 Sep 2014 à 11:52:00 (+0200), Markus Armbruster wrote :
> Cc'ing libvirt following Stefan's lead.
> 
> Benoît Canet  writes:
> 
> > Hi,
> >
> > I collected some items of a cloud provider wishlist regarding I/O accouting.
> 
> Feedback from real power-users, lovely!
> 
> > In a cloud I/O accouting can have 3 purpose: billing, helping the customers
> > and doing metrology to help the cloud provider seeks hidden costs.
> >
> > I'll cover the two former topic in this mail because they are the most 
> > important
> > business wize.
> >
> > 1) prefered place to collect billing IO accounting data:
> > 
> > For billing purpose the collected data must be as close as possible to what 
> > the
> > customer would see by using iostats in his vm.
> 
> Good point.
> 
> > The first conclusion we can draw is that the choice of collecting IO 
> > accouting
> > data used for billing in the block devices models is right.
> 
> Slightly rephrasing: doing I/O accounting in the block device models is
> right for billing.
> 
> There may be other uses for I/O accounting, with different preferences.
> For instance, data on how exactly guest I/O gets translated to host I/O
> as it flows through the nodes in the block graph could be useful.

I think this is the third point that I named as metrology.
Basically it boils down to "Where are the hidden IO costs of the QEMU block 
layer".

> 
> Doesn't diminish the need for accurate billing information, of course.
> 
> > 2) what to do with occurences of rare events:
> > -
> >
> > Another point is that QEMU developpers agree that they don't know which 
> > policy
> > to apply to some I/O accounting events.
> > Must QEMU discard invalid I/O write IO or account them as done ?
> > Must QEMU count a failed read I/O as done ?
> >
> > When discusting this with a cloud provider the following appears:
> > these decisions
> > are really specific to each cloud provider and QEMU should not implement 
> > them.
> 
> Good point, consistent with the old advice to avoid baking policy into
> inappropriately low levels of the stack.
> 
> > The right thing to do is to add accouting counters to collect these events.
> >
> > Moreover these rare events are precious troubleshooting data so it's
> > an additional
> > reason not to toss them.
> 
> Another good point.
> 
> > 3) list of block I/O accouting metrics wished for billing and helping
> > the customers
> > ---
> >
> > Basic I/O accouting data will end up making the customers bills.
> > Extra I/O accouting informations would be a precious help for the cloud 
> > provider
> > to implement a monitoring panel like Amazon Cloudwatch.
> 
> These are the first two from your list of three purposes, i.e. the ones
> you promised to cover here.
> 
> > Here is the list of counters and statitics I would like to help
> > implement in QEMU.
> >
> > This is the most important part of the mail and the one I would like
> > the community
> > review the most.
> >
> > Once this list is settled I would proceed to implement the required
> > infrastructure
> > in QEMU before using it in the device models.
> 
> For context, let me recap how I/O accounting works now.
> 
> The BlockDriverState abstract data type (short: BDS) can hold the
> following accounting data:
> 
> uint64_t nr_bytes[BDRV_MAX_IOTYPE];
> uint64_t nr_ops[BDRV_MAX_IOTYPE];
> uint64_t total_time_ns[BDRV_MAX_IOTYPE];
> uint64_t wr_highest_sector;
> 
> where BDRV_MAX_IOTYPE enumerates read, write, flush.
> 
> wr_highest_sector is a high watermark updated by the block layer as it
> writes sectors.
> 
> The other three are *not* touched by the block layer.  Instead, the
> block layer provides a pair of functions for device models to update
> them:
> 
> void bdrv_acct_start(BlockDriverState *bs, BlockAcctCookie *cookie,
> int64_t bytes, enum BlockAcctType type);
> void bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie);
> 
> bdrv_acct_start() initializes cookie for a read, write, or flush
> operation of a certain size.  The size of a flush is always zero.
> 
> bdrv_acct_done() adds the operations to the BDS's accounting data.
> total_time_ns is incremented by the time between _start() and _done().
> 
> You may call _start() without calling _done().  That's a feature.
> Device models use it to avoid accounting some requests.
> 
> Device models are not supposed to mess with cookie directly, only
> through these two functions.
> 
> Some device models implement accounting, some don't.  The ones that do
> don't agree on how to count invalid guest requests (the ones not passed
> to block layer) and failed requests (passed to block layer and failed
> there).  It's a mess in part caused by us never writing down what
> exactly device models are expected to do.
> 
> Accounting data is used by "q

Re: [libvirt] [Qemu-devel] IO accounting overhaul

[Markus Armbruster]

Benoît Canet  writes:

> The Monday 01 Sep 2014 à 11:52:00 (+0200), Markus Armbruster wrote :
>> Cc'ing libvirt following Stefan's lead.
>> 
>> Benoît Canet  writes:
>> 
>> > Hi,
>> >
>> > I collected some items of a cloud provider wishlist regarding I/O 
>> > accouting.
>> 
>> Feedback from real power-users, lovely!
>> 
>> > In a cloud I/O accouting can have 3 purpose: billing, helping the customers
>> > and doing metrology to help the cloud provider seeks hidden costs.
>> >
>> > I'll cover the two former topic in this mail because they are the
>> > most important
>> > business wize.
>> >
>> > 1) prefered place to collect billing IO accounting data:
>> > 
>> > For billing purpose the collected data must be as close as
>> > possible to what the
>> > customer would see by using iostats in his vm.
>> 
>> Good point.
>> 
>> > The first conclusion we can draw is that the choice of collecting
>> > IO accouting
>> > data used for billing in the block devices models is right.
>> 
>> Slightly rephrasing: doing I/O accounting in the block device models is
>> right for billing.
>> 
>> There may be other uses for I/O accounting, with different preferences.
>> For instance, data on how exactly guest I/O gets translated to host I/O
>> as it flows through the nodes in the block graph could be useful.
>
> I think this is the third point that I named as metrology.
> Basically it boils down to "Where are the hidden IO costs of the QEMU
> block layer".

Understood.

>> Doesn't diminish the need for accurate billing information, of course.
>> 
>> > 2) what to do with occurences of rare events:
>> > -
>> >
>> > Another point is that QEMU developpers agree that they don't know
>> > which policy
>> > to apply to some I/O accounting events.
>> > Must QEMU discard invalid I/O write IO or account them as done ?
>> > Must QEMU count a failed read I/O as done ?
>> >
>> > When discusting this with a cloud provider the following appears:
>> > these decisions
>> > are really specific to each cloud provider and QEMU should not
>> > implement them.
>> 
>> Good point, consistent with the old advice to avoid baking policy into
>> inappropriately low levels of the stack.
>> 
>> > The right thing to do is to add accouting counters to collect these events.
>> >
>> > Moreover these rare events are precious troubleshooting data so it's
>> > an additional
>> > reason not to toss them.
>> 
>> Another good point.
>> 
>> > 3) list of block I/O accouting metrics wished for billing and helping
>> > the customers
>> > ---
>> >
>> > Basic I/O accouting data will end up making the customers bills.
>> > Extra I/O accouting informations would be a precious help for the
>> > cloud provider
>> > to implement a monitoring panel like Amazon Cloudwatch.
>> 
>> These are the first two from your list of three purposes, i.e. the ones
>> you promised to cover here.
>> 
>> > Here is the list of counters and statitics I would like to help
>> > implement in QEMU.
>> >
>> > This is the most important part of the mail and the one I would like
>> > the community
>> > review the most.
>> >
>> > Once this list is settled I would proceed to implement the required
>> > infrastructure
>> > in QEMU before using it in the device models.
>> 
>> For context, let me recap how I/O accounting works now.
>> 
>> The BlockDriverState abstract data type (short: BDS) can hold the
>> following accounting data:
>> 
>> uint64_t nr_bytes[BDRV_MAX_IOTYPE];
>> uint64_t nr_ops[BDRV_MAX_IOTYPE];
>> uint64_t total_time_ns[BDRV_MAX_IOTYPE];
>> uint64_t wr_highest_sector;
>> 
>> where BDRV_MAX_IOTYPE enumerates read, write, flush.
>> 
>> wr_highest_sector is a high watermark updated by the block layer as it
>> writes sectors.
>> 
>> The other three are *not* touched by the block layer.  Instead, the
>> block layer provides a pair of functions for device models to update
>> them:
>> 
>> void bdrv_acct_start(BlockDriverState *bs, BlockAcctCookie *cookie,
>> int64_t bytes, enum BlockAcctType type);
>> void bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie);
>> 
>> bdrv_acct_start() initializes cookie for a read, write, or flush
>> operation of a certain size.  The size of a flush is always zero.
>> 
>> bdrv_acct_done() adds the operations to the BDS's accounting data.
>> total_time_ns is incremented by the time between _start() and _done().
>> 
>> You may call _start() without calling _done().  That's a feature.
>> Device models use it to avoid accounting some requests.
>> 
>> Device models are not supposed to mess with cookie directly, only
>> through these two functions.
>> 
>> Some device models implement accounting, some don't.  The ones that do
>> don't agree on how to count invalid guest requests (the ones not passed
>> to block layer) and failed requests (passed to block

[libvirt] [PATCH 6/6] lxc_container: Resolve Coverity RESOURCE_LEAK

[Wang Rui]

Memory is allocated for 'mnt_src' by VIR_STRDUP in the loop. Next
loop it will be allocated again. So we need to free 'mnt_src'
before continue the loop.

Signed-off-by: Wang Rui 
---
 src/lxc/lxc_container.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 1cf2c8f..2a3ec48 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -886,12 +886,14 @@ static int lxcContainerMountBasicFS(bool userns_enabled,
 if (ret == 0) {
 VIR_DEBUG("Skipping '%s' which isn't mounted in host",
   mnt->dst);
+VIR_FREE(mnt_src);
 continue;
 }
 }
 
 if (mnt->skipUserNS && userns_enabled) {
 VIR_DEBUG("Skipping due to user ns enablement");
+VIR_FREE(mnt_src);
 continue;
 }
 
@@ -930,6 +932,8 @@ static int lxcContainerMountBasicFS(bool userns_enabled,
  MS_BIND|MS_REMOUNT|MS_RDONLY);
 goto cleanup;
 }
+
+VIR_FREE(mnt_src);
 }
 
 rc = 0;
-- 
1.7.12.4


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

[libvirt] [PATCH 5/6] vircgroup: Resolve Coverity RESOURCE_LEAK

[Wang Rui]

Need to free 'root' and 'opts' before 'return -1' if symlink fails.

Signed-off-by: Wang Rui 
---
 src/util/vircgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 8b554a9..a64f081 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3757,7 +3757,7 @@ virCgroupIsolateMount(virCgroupPtr group, const char 
*oldroot,
  _("Unable to symlink directory %s to %s"),
  group->controllers[i].mountPoint,
  group->controllers[i].linkPoint);
-return -1;
+goto cleanup;
 }
 }
 }
-- 
1.7.12.4


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

[libvirt] [PATCH 3/6] remote: Resolve Coverity RESOURCE_LEAK

[Wang Rui]

Need to free 'uri_out' on error path.

Signed-off-by: Wang Rui 
---
 daemon/remote.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 89714ca..0ea2815 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2305,8 +2305,10 @@ remoteDispatchDomainMigratePrepare2(virNetServerPtr 
server ATTRIBUTE_UNUSED,
 rv = 0;
 
  cleanup:
-if (rv < 0)
+if (rv < 0) {
 virNetMessageSaveError(rerr);
+VIR_FREE(uri_out);
+}
 return rv;
 }
 
-- 
1.7.12.4


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

[libvirt] [PATCH 4/6] qemu_process: Resolve Coverity RESOURCE_LEAK

[Wang Rui]

If virSecurityManagerClearSocketLabel() fails, 'agent' won't
be freed before jumping to cleanup.

Signed-off-by: Wang Rui 
---
 src/qemu/qemu_process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f68dfbe..79f4238 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -264,6 +264,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr 
vm)
vm->def) < 0) {
 VIR_ERROR(_("Failed to clear security context for agent for %s"),
   vm->def->name);
+qemuAgentClose(agent);
 goto cleanup;
 }
 
-- 
1.7.12.4


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

[libvirt] [PATCH 0/6] Coverity patches to resolve RESOURCE_LEAK

[Wang Rui]

Another six pathes to fix resource leak.
But this may not be the end.

Wang Rui (6):
  tests: Resolve Coverity RESOURCE_LEAK in commandhelper
  test_conf: Resolve Coverity RESOURCE_LEAK
  remote: Resolve Coverity RESOURCE_LEAK
  qemu_process: Resolve Coverity RESOURCE_LEAK
  vircgroup: Resolve Coverity RESOURCE_LEAK
  lxc_container: Resolve Coverity RESOURCE_LEAK

 daemon/remote.c |  4 +++-
 src/lxc/lxc_container.c |  4 
 src/qemu/qemu_process.c |  1 +
 src/util/vircgroup.c|  2 +-
 tests/commandhelper.c   | 15 +--
 tests/test_conf.c   |  4 ++--
 6 files changed, 20 insertions(+), 10 deletions(-)

-- 
1.7.12.4


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

[libvirt] [PATCH 2/6] test_conf: Resolve Coverity RESOURCE_LEAK

[Wang Rui]

If the condition 'ret < 0' is true, the code will jump to
'cleanup' and 'conf' won't be freed.

Signed-off-by: Wang Rui 
---
 tests/test_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_conf.c b/tests/test_conf.c
index 05704df..4d05d8d 100644
--- a/tests/test_conf.c
+++ b/tests/test_conf.c
@@ -11,7 +11,7 @@
 int main(int argc, char **argv)
 {
 int ret, exit_code = EXIT_FAILURE;
-virConfPtr conf;
+virConfPtr conf = NULL;
 int len = 1;
 char *buffer = NULL;
 
@@ -34,7 +34,6 @@ int main(int argc, char **argv)
 fprintf(stderr, "Failed to serialize %s back\n", argv[1]);
 goto cleanup;
 }
-virConfFree(conf);
 if (fwrite(buffer, 1, len, stdout) != len) {
 fprintf(stderr, "Write failed: %s\n", strerror(errno));
 goto cleanup;
@@ -44,5 +43,6 @@ int main(int argc, char **argv)
 
  cleanup:
 VIR_FREE(buffer);
+virConfFree(conf);
 return exit_code;
 }
-- 
1.7.12.4


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

[libvirt] [PATCH 1/6] tests: Resolve Coverity RESOURCE_LEAK in commandhelper

[Wang Rui]

Coverity determined that 'log' and 'newenv' were not freed in
some cases. Free them in 'error' branch and normal branch.

Signed-off-by: Wang Rui 
---
 tests/commandhelper.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 796b89d..0bba0d6 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -61,7 +61,7 @@ int main(int argc, char **argv) {
 size_t i, n;
 int open_max;
 char **origenv;
-char **newenv;
+char **newenv = NULL;
 char *cwd;
 FILE *log = fopen(abs_builddir "/commandhelper.log", "w");
 
@@ -80,7 +80,7 @@ int main(int argc, char **argv) {
 }
 
 if (VIR_ALLOC_N_QUIET(newenv, n) < 0)
-return EXIT_FAILURE;
+goto error;
 
 origenv = environ;
 n = i = 0;
@@ -100,7 +100,7 @@ int main(int argc, char **argv) {
 
 open_max = sysconf(_SC_OPEN_MAX);
 if (open_max < 0)
-return EXIT_FAILURE;
+goto error;
 for (i = 0; i < open_max; i++) {
 int f;
 int closed;
@@ -114,15 +114,13 @@ int main(int argc, char **argv) {
 
 fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no");
 if (!(cwd = getcwd(NULL, 0)))
-return EXIT_FAILURE;
+goto error;
 if (strlen(cwd) > strlen(".../commanddata") &&
 STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata"))
 strcpy(cwd, ".../commanddata");
 fprintf(log, "CWD:%s\n", cwd);
 VIR_FREE(cwd);
 
-VIR_FORCE_FCLOSE(log);
-
 if (argc > 1 && STREQ(argv[1], "--close-stdin")) {
 if (freopen("/dev/null", "r", stdin) != stdin)
 goto error;
@@ -154,9 +152,14 @@ int main(int argc, char **argv) {
 fprintf(stderr, "END STDERR\n");
 fflush(stderr);
 
+VIR_FORCE_FCLOSE(log);
+VIR_FREE(newenv);
+
 return EXIT_SUCCESS;
 
  error:
+VIR_FORCE_FCLOSE(log);
+VIR_FREE(newenv);
 return EXIT_FAILURE;
 }
 
-- 
1.7.12.4


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

Re: [libvirt] [PATCH v4 2/3] qemu: Implement extended loader and nvram

[Michal Privoznik]

On 22.08.2014 18:48, Eric Blake wrote:

On 08/21/2014 02:50 AM, Michal Privoznik wrote:

QEMU now supports UEFI with the following command line:

   -drive 
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
   -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \

where the first line reflects  and the second one .
Moreover, these two lines obsoletes the -bios argument.


s/obsoletes/obsolete/



Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.

Signed-off-by: Michal Privoznik 
---



+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+/* UEFI is supported only for x86_64 currently */
+if (def->os.arch != VIR_ARCH_X86_64) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("pflash is not supported for %s guest 
achitecture"),


s/achitecture/architecture/



+
+if (loader->readonly) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support passing "
+ "readonly attribute"));
+goto cleanup;
+}
+
+virBufferAsprintf(&buf, ",readonly=%s",
+  virTristateSwitchTypeToString(loader->readonly));
+}
+
+virCommandAddArg(cmd, "-drive");
+virCommandAddArgBuffer(cmd, &buf);
+
+if (loader->nvram) {
+virBufferFreeAndReset(&buf);
+virBufferAsprintf(&buf,
+  "file=%s,if=pflash,format=raw,unit=%d",
+  loader->nvram, unit);


Hmm.  Here, it looks like readonly='on' is supported ONLY for
type='pflash', and not for type='rom'.  In that case, I'd write the .rng
of patch 1 as (rough draft):


   
  
   
 
   rom
 
   
 
  
   
 pflash
   
   
 
   
   
 nvram stuff...
   
 
   
   



While this can be correct I think having wider XML schema then the code 
is just okay. Keeping the schema readable wins over tightening all the 
narrow cases for me.


Michal

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

Re: [libvirt] [PATCH v4 1/3] conf: Extend and introduce

[Michal Privoznik]

On 22.08.2014 18:41, Eric Blake wrote:

On 08/21/2014 02:50 AM, Michal Privoznik wrote:

Up to now, users can configure BIOS via the  element. With
the upcoming implementation of UEFI this is not enough as BIOS and
UEFI are conceptually different. For instance, while BIOS is ROM, UEFI
is programmable flash (although all writes to code section are
denied). Therefore we need new attribute @type which will
differentiate the two. Then, new attribute @readonly is introduced to
reflect the fact that some images are RO.

Moreover, the OVMF (which is going to be used mostly), works in two
modes:
1) Code and UEFI variable store is mixed in one file.
2) Code and UEFI variable store is separated in two files

The latter has advantage of updating the UEFI code without losing the
configuration. However, in order to represent the latter case we need
yet another XML element: . Currently, it has no additional
attributes, it's just a bare element containing path to the variable
store file.




+++ b/docs/formatdomain.html.in
@@ -102,7 +102,8 @@
...

  hvm
-/usr/lib/xen/boot/hvmloader
+/usr/lib/xen/boot/hvmloader


readonly='yes' is a bit more typical of other XML constructs.


I didn't know that. Everything I found was just a bare  
element. But okay, I'm fine with 'yes|no' too.





+/var/lib/libvirt/nvram/guest_VARS.fd


You chose  to be a sibling, rather than a child, of .  Is
it legal to have  in isolation, or can it only appear when
 is present?  If the former, then you are okay; if the latter,
then I'd rather see it as a child than a sibling.


  
  
  
@@ -129,7 +130,21 @@
  used to assist the domain creation process. It is used by Xen
  fully virtualized domains as well as setting the QEMU BIOS file
  path for QEMU/KVM domains. Xen since 0.1.0,
-QEMU/KVM since 0.9.12
+QEMU/KVM since 0.9.12 Then, Since


s/Since/since/, because you are using it in the middle of a sentence


+1.2.8 it's possible for the element to have two
+optional attributes: readonly (accepted values are
+on and off) to reflect the fact that the
+image should be writable or read-only.


Again, yes/no might be more consistent than on/off


The second attribute
+type accepts values rom and
+pflash. It tells the hypervisor where in the guest
+memory the file should be mapped.  For instance, if the loader
+path points to an UEFI image, type should be
+pflash.
+  nvram
+  Some UEFI firmwares may want to use a non-volatile memory to store
+some variables. In the host, this is represented as a file and the
+path to the file is stored in this element. Since
+1.2.8


Is this going to bite us in the future?  What if we want to store the
file on a networked device, such as via gluster or nbd?  I'm wondering if:


   


is a better representation, because that way, we could also do:


...
 
   



Who would ever want to store UEFI image/varstore on gluster? It's 2M in 
total after all from which the nvram is 128KiB.




Also, by reusing a virStorageSourcePtr to store the nvram location,
rather than limiting to just a file name, we can ensure we do proper
SELinux labeling of the file, and allow the user the ability to
overwrite what label we would otherwise generate.


It doesn't make much sense to me to have different security label on the 
nvram file than the qemu process is going to have. Thus I don't think we 
should even allow arbitrary labels here.






+++ b/docs/schemas/domaincommon.rng
@@ -242,6 +242,27 @@

  

+
+  
+
+  on
+  off
+
+  
+
+
+  
+
+  rom
+  pflash
+
+  
+
+
+  
+
+
+  
  


This matches your docs, whether or not we decide to change the design.
And at any rate, the existing  element is just as hard-coded to
a local filename as your new nvram, so if we add type='file' support,
we'd want it on both places at once, so probably fine to defer that to a
later day if someone actually needs it.  So for now, I'm okay living
with the design of just a string (but would still like to see the on/off
changed to yes/no), since it's better to get OVMF usage enabled than to
wait for a more complicated virStorageSource solution.



+static int
+virDomainLoaderDefParseXML(xmlNodePtr node,
+   virDomainLoaderDefPtr loader)
+{
+int ret = -1;
+char *readonly_str = NULL;
+char *type_str = NULL;
+
+readonly_str = virXMLPropString(node, "readonly");
+type_str = virXMLPropString(no

[libvirt] [PATCH for 1.2.8] selinux: properly label tap FDs with imagelabel

[Martin Kletzander]

The cleanup in commit cf976d9d used secdef->label to label the tap
FDs, but that is not possible since it's process-only label (svirt_t)
and not a object label (e.g. svirt_image_t).  Starting a domain failed
with EPERM, but simply using secdef->label instead fixes it.

Signed-off-by: Martin Kletzander 
---
 src/security/security_selinux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 5d18493..e8c13db 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2340,7 +2340,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED,
 if (!secdef || !secdef->label)
 return 0;

-return virSecuritySELinuxFSetFilecon(fd, secdef->label);
+return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel);
 }

 static char *
-- 
2.1.0

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

Re: [libvirt] [PATCH for 1.2.8] selinux: properly label tap FDs with imagelabel

[Pavel Hrdina]

On 09/01/2014 03:31 PM, Martin Kletzander wrote:
> The cleanup in commit cf976d9d used secdef->label to label the tap
> FDs, but that is not possible since it's process-only label (svirt_t)
> and not a object label (e.g. svirt_image_t).  Starting a domain failed
> with EPERM, but simply using secdef->label instead fixes it.

s/label/imagelabel/

> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/security/security_selinux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 5d18493..e8c13db 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2340,7 +2340,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr 
> mgr ATTRIBUTE_UNUSED,
>  if (!secdef || !secdef->label)
>  return 0;
> 
> -return virSecuritySELinuxFSetFilecon(fd, secdef->label);
> +return virSecuritySELinuxFSetFilecon(fd, secdef->imagelabel);
>  }
> 
>  static char *
> 

ACK with that change

Pavel

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

Re: [libvirt] [Qemu-devel] IO accounting overhaul

[Benoît Canet]

The Monday 01 Sep 2014 à 13:41:01 (+0200), Markus Armbruster wrote :
> Benoît Canet  writes:
> 
> > The Monday 01 Sep 2014 à 11:52:00 (+0200), Markus Armbruster wrote :
> >> Cc'ing libvirt following Stefan's lead.
> >> 
> >> Benoît Canet  writes:
> >> 
> >> > Hi,
> >> >
> >> > I collected some items of a cloud provider wishlist regarding I/O 
> >> > accouting.
> >> 
> >> Feedback from real power-users, lovely!
> >> 
> >> > In a cloud I/O accouting can have 3 purpose: billing, helping the 
> >> > customers
> >> > and doing metrology to help the cloud provider seeks hidden costs.
> >> >
> >> > I'll cover the two former topic in this mail because they are the
> >> > most important
> >> > business wize.
> >> >
> >> > 1) prefered place to collect billing IO accounting data:
> >> > 
> >> > For billing purpose the collected data must be as close as
> >> > possible to what the
> >> > customer would see by using iostats in his vm.
> >> 
> >> Good point.
> >> 
> >> > The first conclusion we can draw is that the choice of collecting
> >> > IO accouting
> >> > data used for billing in the block devices models is right.
> >> 
> >> Slightly rephrasing: doing I/O accounting in the block device models is
> >> right for billing.
> >> 
> >> There may be other uses for I/O accounting, with different preferences.
> >> For instance, data on how exactly guest I/O gets translated to host I/O
> >> as it flows through the nodes in the block graph could be useful.
> >
> > I think this is the third point that I named as metrology.
> > Basically it boils down to "Where are the hidden IO costs of the QEMU
> > block layer".
> 
> Understood.
> 
> >> Doesn't diminish the need for accurate billing information, of course.
> >> 
> >> > 2) what to do with occurences of rare events:
> >> > -
> >> >
> >> > Another point is that QEMU developpers agree that they don't know
> >> > which policy
> >> > to apply to some I/O accounting events.
> >> > Must QEMU discard invalid I/O write IO or account them as done ?
> >> > Must QEMU count a failed read I/O as done ?
> >> >
> >> > When discusting this with a cloud provider the following appears:
> >> > these decisions
> >> > are really specific to each cloud provider and QEMU should not
> >> > implement them.
> >> 
> >> Good point, consistent with the old advice to avoid baking policy into
> >> inappropriately low levels of the stack.
> >> 
> >> > The right thing to do is to add accouting counters to collect these 
> >> > events.
> >> >
> >> > Moreover these rare events are precious troubleshooting data so it's
> >> > an additional
> >> > reason not to toss them.
> >> 
> >> Another good point.
> >> 
> >> > 3) list of block I/O accouting metrics wished for billing and helping
> >> > the customers
> >> > ---
> >> >
> >> > Basic I/O accouting data will end up making the customers bills.
> >> > Extra I/O accouting informations would be a precious help for the
> >> > cloud provider
> >> > to implement a monitoring panel like Amazon Cloudwatch.
> >> 
> >> These are the first two from your list of three purposes, i.e. the ones
> >> you promised to cover here.
> >> 
> >> > Here is the list of counters and statitics I would like to help
> >> > implement in QEMU.
> >> >
> >> > This is the most important part of the mail and the one I would like
> >> > the community
> >> > review the most.
> >> >
> >> > Once this list is settled I would proceed to implement the required
> >> > infrastructure
> >> > in QEMU before using it in the device models.
> >> 
> >> For context, let me recap how I/O accounting works now.
> >> 
> >> The BlockDriverState abstract data type (short: BDS) can hold the
> >> following accounting data:
> >> 
> >> uint64_t nr_bytes[BDRV_MAX_IOTYPE];
> >> uint64_t nr_ops[BDRV_MAX_IOTYPE];
> >> uint64_t total_time_ns[BDRV_MAX_IOTYPE];
> >> uint64_t wr_highest_sector;
> >> 
> >> where BDRV_MAX_IOTYPE enumerates read, write, flush.
> >> 
> >> wr_highest_sector is a high watermark updated by the block layer as it
> >> writes sectors.
> >> 
> >> The other three are *not* touched by the block layer.  Instead, the
> >> block layer provides a pair of functions for device models to update
> >> them:
> >> 
> >> void bdrv_acct_start(BlockDriverState *bs, BlockAcctCookie *cookie,
> >> int64_t bytes, enum BlockAcctType type);
> >> void bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie);
> >> 
> >> bdrv_acct_start() initializes cookie for a read, write, or flush
> >> operation of a certain size.  The size of a flush is always zero.
> >> 
> >> bdrv_acct_done() adds the operations to the BDS's accounting data.
> >> total_time_ns is incremented by the time between _start() and _done().
> >> 
> >> You may call _start() without calling _done().  That's a feature.
> >> Device models use it to avoid 

Re: [libvirt] [python PATCH 3/5] API: Implement bindings for virConnectGetAllDomainStats

[Pavel Hrdina]

On 08/28/2014 06:32 PM, Peter Krempa wrote:
> Implement the function by returning a list of tuples instead the array
> of virDomainStatsRecords and store the typed parameters as dict.
> ---
>  generator.py   |  1 +
>  libvirt-override-virConnect.py | 53 
>  libvirt-override.c | 94 
> ++
>  3 files changed, 148 insertions(+)
> 

ACK

Pavel

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

Re: [libvirt] [PATCH v3 00/18] Implement virDomainBlockCopy

[Jiri Denemark]

On Mon, Sep 01, 2014 at 09:39:35 +0200, Jiri Denemark wrote:
> On Sat, Aug 30, 2014 at 22:02:18 -0600, Eric Blake wrote:
> > Took me longer than I wanted to get v3 posted.  There's lots of
> > new patches in this version, based on feedback on v2.
> > 
> > Among other things, the bandwidth of virDomainBlockCopy is in
> > bytes/s, and all the remaining interfaces are updated to use a
> > flags value to decide whether to use bytes/s instead of the
> > back-compat MiB/s.
> > 
> > I really want patch 1 in 1.2.8; pasth 2-17 would be nice but
> > it may be better to delay to later, and I still don't have
> > virsh updated to cover the changed proposed in patch 18 so that
> > one should probably wait.
> 
> Patch 1 is easy and safe to be included in 1.2.8 but I think it's way
> too late for the rest. However, since the implementation will be missing
> from 1.2.8, I think the best solution would be to just revert the patch
> adding this public API and let it slip to the next release.

On the other side, we already did a mistake and pushed API when its
implementation was not ready yet and given that the API doesn't seem to
need any changes (except for the tiny one in 1/18), there's no strong
reason to revert it (except of for it being unsupported by all drivers).

It would be nice to at least have the remote driver implementation in
place but it's probably too late for that too.

Jirka

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

Re: [libvirt] [python PATCH 4/5] API: Implement bindings for virDomainListGetStats

[Pavel Hrdina]

On 08/28/2014 06:32 PM, Peter Krempa wrote:
> Implement the function by returning a list of tuples instead the array
> of virDomainStatsRecords and store the typed parameters as dict.
> ---
>  generator.py   |  1 +
>  libvirt-override-virConnect.py | 47 +++
>  libvirt-override.c | 50 
> ++
>  3 files changed, 98 insertions(+)
> 
> diff --git a/generator.py b/generator.py
> index 9addb89..3642838 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -508,6 +508,7 @@ skip_function = (
>  'virConnectListAllNWFilters', # overridden in virConnect.py
>  'virConnectListAllSecrets', # overridden in virConnect.py
>  'virConnectGetAllDomainStats', # overridden in virConnect.py
> +'virDomainListGetStats', # overriden in virConnect.py
> 
>  'virStreamRecvAll', # Pure python libvirt-override-virStream.py
>  'virStreamSendAll', # Pure python libvirt-override-virStream.py
> diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py
> index c4c400a..218f266 100644
> --- a/libvirt-override-virConnect.py
> +++ b/libvirt-override-virConnect.py
> @@ -436,3 +436,50 @@
>  retlist.append(record)
> 
>  return retlist
> +
> +def domainListGetStats(self, doms, stats=0, flags=0):
> +""" Query statistics for given domains.
> +
> +Report statistics of various parameters for a running VM according 
> to @stats
> +field. The statistics are returned as an array of structures for 
> each queried
> +domain. The structure contains an array of typed parameters 
> containing the
> +individual statistics. The typed parameter name for each statistic 
> field
> +consists of a dot-separated string containing name of the requested 
> group
> +followed by a group specific description of the statistic value.
> +
> +The statistic groups are enabled using the @stats parameter which is 
> a
> +binary-OR of enum virDomainStatsTypes. The following groups are 
> available
> +(although not necessarily implemented for each hypervisor):
> +
> +VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering 
> that
> +state. The typed parameter keys are in this format:
> +"state.state" - state of the VM, returned as int from virDomainState 
> enum
> +"state.reason" - reason for entering given state, returned as int 
> from
> + virDomain*Reason enum corresponding to given state.
> +
> +Using 0 for @stats returns all stats groups supported by the given
> +hypervisor.
> +
> +Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags 
> makes
> +the function return error in case some of the stat types in @stats 
> were
> +not recognized by the daemon.
> +
> +Get statistics about domains provided as a list in @doms. @stats is
> +a bit field selecting requested statistics types."""
> +domlist = list()
> +for dom in doms:
> +if not isinstance(dom, virDomain):
> +raise libvirtError("domain list contains non-domain 
> elements", conn=self)
> +
> +domlist.append(dom._o)
> +
> +ret = libvirtmod.virDomainListGetStats(self._o, domlist, stats, 
> flags)
> +if ret is None:
> +raise libvirtError("virDomainListGetStats() failed", conn=self)
> +
> +retlist = list()
> +for elem in ret:
> +record = (virDomain(self, _obj=elem[0]) , elem[1])
> +retlist.append(record)
> +
> +return retlist

The function 'domainListGetStats' should be implemented in
libvirt-override-virDomain.py as 'listGetStats'.

> diff --git a/libvirt-override.c b/libvirt-override.c
> index df4f15b..7e9f570 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -5052,6 +5052,55 @@ libvirt_virConnectGetAllDomainStats(PyObject *self 
> ATTRIBUTE_UNUSED,
> 
>  return py_retval;
>  }
> +
> +
> +static PyObject *
> +libvirt_virDomainListGetStats(PyObject *self ATTRIBUTE_UNUSED,
> +  PyObject *args)
> +{
> +PyObject *pyobj_conn;
> +PyObject *py_retval;
> +PyObject *py_domlist;
> +virConnectPtr conn;
> +virDomainStatsRecordPtr *records;

Set records to NULL to make 'virDomainStatsRecordListFree' happy if the
'virDomainListGetStats' fails.

> +virDomainPtr *doms = NULL;
> +int nrecords;
> +int ndoms;
> +size_t i;
> +unsigned int flags;
> +unsigned int stats;
> +
> +if (!PyArg_ParseTuple(args, (char *)"OOii:virDomainListGetStats",
> +  &pyobj_conn, &py_domlist, &stats, &flags))
> +return NULL;
> +conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
> +
> +if (PyList_Check(py_domlist)) {
> +ndoms = PyList_Size(py_domlist);
> +
> +if (VIR_ALLOC_N(doms, ndoms + 1) < 0)
> +  

Re: [libvirt] [PATCH v3 01/18] blockcopy: allow larger buf-size

[Jiri Denemark]

On Sat, Aug 30, 2014 at 22:02:19 -0600, Eric Blake wrote:
> While qemu definitely caps granularity to 64 MiB, it places no
> limits on buf-size.  On a machine beefy enough for lots of
> memory, a buf-size larger than 2 GiB is feasible, so we should
> pass a 64-bit parameter.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_COPY_BUF_SIZE):
> Allow 64 bits.
> 
> Signed-off-by: Eric Blake 
> ---
>  include/libvirt/libvirt.h.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 9358314..a64f597 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2678,8 +2678,8 @@ typedef enum {
>   * VIR_DOMAIN_BLOCK_COPY_BUF_SIZE:
>   * Macro for the virDomainBlockCopy buffer size tunable: it represents
>   * how much data in bytes can be in flight between source and destination,
> - * as an unsigned int. Specifying 0 is the same as omitting this parameter,
> - * to request the hypervisor default.
> + * as an unsigned long long. Specifying 0 is the same as omitting this
> + * parameter, to request the hypervisor default.
>   */
>  #define VIR_DOMAIN_BLOCK_COPY_BUF_SIZE "buf-size"

ACK, we need this in 1.2.8.

Jirka

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

[libvirt] [PATCH 4/6] qemu: Transfer migration statistics to destination

[Jiri Denemark]

When migrating a transient domain or with VIR_MIGRATE_UNDEFINE_SOURCE
flag, the domain may disappear from source host. And so will migration
statistics associated with the domain. We need to transfer the
statistics at the end of a migration so that they can be queried at the
destination host.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 190 +-
 1 file changed, 187 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 208a21f..f1b3d50 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -80,6 +80,7 @@ enum qemuMigrationCookieFlags {
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
 QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
 QEMU_MIGRATION_COOKIE_FLAG_NBD,
+QEMU_MIGRATION_COOKIE_FLAG_STATS,
 
 QEMU_MIGRATION_COOKIE_FLAG_LAST
 };
@@ -91,7 +92,8 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag,
   "lockstate",
   "persistent",
   "network",
-  "nbd");
+  "nbd",
+  "statistics");
 
 enum qemuMigrationCookieFeatures {
 QEMU_MIGRATION_COOKIE_GRAPHICS  = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
@@ -99,6 +101,7 @@ enum qemuMigrationCookieFeatures {
 QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << 
QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
 QEMU_MIGRATION_COOKIE_NETWORK = (1 << QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
 QEMU_MIGRATION_COOKIE_NBD = (1 << QEMU_MIGRATION_COOKIE_FLAG_NBD),
+QEMU_MIGRATION_COOKIE_STATS = (1 << QEMU_MIGRATION_COOKIE_FLAG_STATS),
 };
 
 typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
@@ -169,6 +172,9 @@ struct _qemuMigrationCookie {
 
 /* If (flags & QEMU_MIGRATION_COOKIE_NBD) */
 qemuMigrationCookieNBDPtr nbd;
+
+/* If (flags & QEMU_MIGRATION_COOKIE_STATS) */
+qemuDomainJobInfoPtr jobInfo;
 };
 
 static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr 
grap)
@@ -533,6 +539,25 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig,
 }
 
 
+static int
+qemuMigrationCookieAddStatistics(qemuMigrationCookiePtr mig,
+ virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+
+if (!priv->job.completed)
+return 0;
+
+if (!mig->jobInfo && VIR_ALLOC(mig->jobInfo) < 0)
+return -1;
+
+*mig->jobInfo = *priv->job.completed;
+mig->flags |= QEMU_MIGRATION_COOKIE_STATS;
+
+return 0;
+}
+
+
 static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
  
qemuMigrationCookieGraphicsPtr grap)
 {
@@ -589,6 +614,81 @@ qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf,
 }
 
 
+static void
+qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf,
+   qemuDomainJobInfoPtr jobInfo)
+{
+qemuMonitorMigrationStatus *status = &jobInfo->status;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_TIME_ELAPSED,
+  jobInfo->timeElapsed);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_TIME_REMAINING,
+  jobInfo->timeRemaining);
+if (status->downtime_set)
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_DOWNTIME,
+  status->downtime);
+
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_TOTAL,
+  status->ram_total);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+  status->ram_transferred);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_REMAINING,
+  status->ram_remaining);
+
+if (status->ram_duplicate_set) {
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_CONSTANT,
+  status->ram_duplicate);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_NORMAL,
+  status->ram_normal);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_MEMORY_NORMAL_BYTES,
+  status->ram_normal_bytes);
+}
+
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_DISK_TOTAL,
+  status->disk_total);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_DISK_PROCESSED,
+  status->disk_transferred);
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+  VIR_DOMAIN_JOB_DISK_REMAINING,
+  status->disk_remaining);
+
+if (status->xbzrle_set) {
+virBufferAsprintf(buf, "<%1$s>%2$llu\n",
+

[libvirt] [PATCH 6/6] qemu: Transfer recomputed stats back to source

[Jiri Denemark]

After previous commit, migration statistics on source and destination
hosts are not equal because destination updated time statistics. Let's
send the result back so that the same data can be queried on both end of
a migration.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 43b42ac..873a756 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3019,9 +3019,27 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
  ? QEMU_MIGRATION_PHASE_CONFIRM3
  : QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED);
 
-if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, 0)))
+if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
+   QEMU_MIGRATION_COOKIE_STATS)))
 goto cleanup;
 
+/* Update total times with the values sent by the destination daemon */
+if (mig->jobInfo) {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+if (priv->job.completed) {
+qemuDomainJobInfoPtr jobInfo = priv->job.completed;
+if (mig->jobInfo->status.downtime_set) {
+jobInfo->status.downtime = mig->jobInfo->status.downtime;
+jobInfo->status.downtime_set = true;
+}
+if (mig->jobInfo->timeElapsed)
+jobInfo->timeElapsed = mig->jobInfo->timeElapsed;
+} else {
+priv->job.completed = mig->jobInfo;
+mig->jobInfo = NULL;
+}
+}
+
 if (flags & VIR_MIGRATE_OFFLINE)
 goto done;
 
@@ -4860,7 +4878,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
  VIR_DOMAIN_EVENT_STOPPED_FAILED);
 }
 
-if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) < 
0)
+if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
+QEMU_MIGRATION_COOKIE_STATS) < 0)
 VIR_WARN("Unable to encode migration cookie");
 
  endjob:
-- 
2.1.0

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

[libvirt] [PATCH 5/6] qemu: Recompute downtime and total time when migration completes

[Jiri Denemark]

Total time of a migration and total downtime transfered from a source to
a destination host do not count with the transfer time to the
destination host and with the time elapsed before guest CPUs are
resumed. Thus, source libvirtd remembers when migration started and when
guest CPUs were paused. Both timestamps are transferred to destination
libvirtd which uses them to compute total migration time and total
downtime. This, obviously, requires clock to be synchronized between the
two hosts. The reported times are useless otherwise but they would be
equally useless if we didn't do this recomputation so don't lose
anything by doing it.

Signed-off-by: Jiri Denemark 
---
 src/libvirt.c |  5 -
 src/qemu/qemu_domain.c| 28 
 src/qemu/qemu_domain.h|  2 ++
 src/qemu/qemu_migration.c | 15 ++-
 src/qemu/qemu_process.c   |  9 -
 tools/virsh.pod   |  5 -
 6 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 6fa0a6b..61d0543 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17581,7 +17581,10 @@ virDomainGetJobInfo(virDomainPtr domain, 
virDomainJobInfoPtr info)
  * return statistics about a recently completed job. Specifically, this
  * flag may be used to query statistics of a completed incoming migration.
  * Statistics of a completed job are automatically destroyed once read or
- * when libvirtd is restarted.
+ * when libvirtd is restarted. Note that time information returned for
+ * completed migrations may be completely irrelevant unless both source and
+ * destination hosts have synchronized time (i.e., NTP daemon is running on
+ * both of them).
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 18a3761..cec7828 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -222,11 +222,39 @@ qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo)
 if (virTimeMillisNow(&now) < 0)
 return -1;
 
+if (now < jobInfo->started) {
+VIR_WARN("Async job starts in the future");
+jobInfo->started = 0;
+return 0;
+}
+
 jobInfo->timeElapsed = now - jobInfo->started;
 return 0;
 }
 
 int
+qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo)
+{
+unsigned long long now;
+
+if (!jobInfo->stopped)
+return 0;
+
+if (virTimeMillisNow(&now) < 0)
+return -1;
+
+if (now < jobInfo->stopped) {
+VIR_WARN("Guest's CPUs stopped in the future");
+jobInfo->stopped = 0;
+return 0;
+}
+
+jobInfo->status.downtime = now - jobInfo->stopped;
+jobInfo->status.downtime_set = true;
+return 0;
+}
+
+int
 qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
 virDomainJobInfoPtr info)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 365238b..435a22b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -105,6 +105,7 @@ typedef qemuDomainJobInfo *qemuDomainJobInfoPtr;
 struct _qemuDomainJobInfo {
 virDomainJobType type;
 unsigned long long started; /* When the async job started */
+unsigned long long stopped; /* When the domain's CPUs were stopped */
 /* Computed values */
 unsigned long long timeElapsed;
 unsigned long long timeRemaining;
@@ -390,6 +391,7 @@ bool qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv,
   bool reportError);
 
 int qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo);
+int qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo);
 int qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
 virDomainJobInfoPtr info);
 int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f1b3d50..43b42ac 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -623,6 +623,9 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf,
 virBufferAddLit(buf, "\n");
 virBufferAdjustIndent(buf, 2);
 
+virBufferAsprintf(buf, "%llu\n", jobInfo->started);
+virBufferAsprintf(buf, "%llu\n", jobInfo->stopped);
+
 virBufferAsprintf(buf, "<%1$s>%2$llu\n",
   VIR_DOMAIN_JOB_TIME_ELAPSED,
   jobInfo->timeElapsed);
@@ -891,6 +894,9 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr 
ctxt)
 status = &jobInfo->status;
 jobInfo->type = VIR_DOMAIN_JOB_COMPLETED;
 
+virXPathULongLong("string(./started[1])", ctxt, &jobInfo->started);
+virXPathULongLong("string(./stopped[1])", ctxt, &jobInfo->stopped);
+
 virXPathULongLong("string(./" VIR_DOMAIN_JOB_TIME_ELAPSED "[1])",
   ctxt, &jobInfo->timeElapsed);
 virXPathULongLong("string(./" VIR_DOMAIN_JOB_TIME_REMAINING "[1])",
@@ -2015,6 +2021,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 }
 
 if 

[libvirt] [PATCH 3/6] virsh: Add support for completed job stats

[Jiri Denemark]

New --completed flag for virsh domjobinfo command.

Signed-off-by: Jiri Denemark 
---
 tools/virsh-domain.c | 27 ---
 tools/virsh.pod  |  5 +++--
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c75cd73..1ff264e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5164,6 +5164,10 @@ static const vshCmdOptDef opts_domjobinfo[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("domain name, id or uuid")
 },
+{.name = "completed",
+ .type = VSH_OT_BOOL,
+ .help = N_("return statistics of a recently completed job")
+},
 {.name = NULL}
 };
 
@@ -5195,14 +5199,18 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 virTypedParameterPtr params = NULL;
 int nparams = 0;
 unsigned long long value;
+unsigned int flags = 0;
 int rc;
 
 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return false;
 
+if (vshCommandOptBool(cmd, "completed"))
+flags |= VIR_DOMAIN_JOB_STATS_COMPLETED;
+
 memset(&info, 0, sizeof(info));
 
-rc = virDomainGetJobStats(dom, &info.type, ¶ms, &nparams, 0);
+rc = virDomainGetJobStats(dom, &info.type, ¶ms, &nparams, flags);
 if (rc == 0) {
 if (virTypedParamsGetULLong(params, nparams,
 VIR_DOMAIN_JOB_TIME_ELAPSED,
@@ -5239,6 +5247,11 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 &info.fileRemaining) < 0)
 goto save_error;
 } else if (last_error->code == VIR_ERR_NO_SUPPORT) {
+if (flags) {
+vshError(ctl, "%s", _("Optional flags are not supported by the "
+  "daemon"));
+goto cleanup;
+}
 vshDebug(ctl, VSH_ERR_DEBUG, "detailed statistics not supported\n");
 vshResetLibvirtError();
 rc = virDomainGetJobInfo(dom, &info);
@@ -5249,7 +5262,9 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 vshPrint(ctl, "%-17s %-12s\n", _("Job type:"),
  vshDomainJobToString(info.type));
 if (info.type != VIR_DOMAIN_JOB_BOUNDED &&
-info.type != VIR_DOMAIN_JOB_UNBOUNDED) {
+info.type != VIR_DOMAIN_JOB_UNBOUNDED &&
+(!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) ||
+ info.type != VIR_DOMAIN_JOB_COMPLETED)) {
 ret = true;
 goto cleanup;
 }
@@ -5314,7 +5329,13 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
   &value)) < 0) {
 goto save_error;
 } else if (rc) {
-vshPrint(ctl, "%-17s %-12llu ms\n", _("Expected downtime:"), value);
+if (info.type == VIR_DOMAIN_JOB_COMPLETED) {
+vshPrint(ctl, "%-17s %-12llu ms\n",
+ _("Total downtime:"), value);
+} else {
+vshPrint(ctl, "%-17s %-12llu ms\n",
+ _("Expected downtime:"), value);
+}
 }
 
 if ((rc = virTypedParamsGetULLong(params, nparams,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ea9267e..3c71db9 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1112,9 +1112,10 @@ Convert a domain name (or UUID) to a domain id
 
 Abort the currently running domain job.
 
-=item B I
+=item B I [I<--completed>]
 
-Returns information about jobs running on a domain.
+Returns information about jobs running on a domain. I<--completed> tells
+virsh to return information about a recently finished job.
 
 =item B I
 
-- 
2.1.0

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

[libvirt] [PATCH 1/6] Refactor job statistics

[Jiri Denemark]

Job statistics data were tracked in several structures and variables.
Let's make a new qemuDomainJobInfo structure which can be used as a
single source of statistics data as a preparation for storing data about
completed a job.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_domain.c| 157 --
 src/qemu/qemu_domain.h|  24 ++-
 src/qemu/qemu_driver.c| 119 ---
 src/qemu/qemu_migration.c |  72 -
 4 files changed, 213 insertions(+), 159 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e9506e0..2c709e9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -163,11 +163,9 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->asyncOwner = 0;
 job->phase = 0;
 job->mask = DEFAULT_JOB_MASK;
-job->start = 0;
 job->dump_memory_only = false;
 job->asyncAbort = false;
-memset(&job->status, 0, sizeof(job->status));
-memset(&job->info, 0, sizeof(job->info));
+VIR_FREE(job->current);
 }
 
 void
@@ -200,6 +198,7 @@ qemuDomainObjTransferJob(virDomainObjPtr obj)
 static void
 qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
 {
+VIR_FREE(priv->job.current);
 virCondDestroy(&priv->job.cond);
 virCondDestroy(&priv->job.asyncCond);
 }
@@ -211,6 +210,150 @@ qemuDomainTrackJob(qemuDomainJob job)
 }
 
 
+int
+qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo)
+{
+unsigned long long now;
+
+if (!jobInfo->started)
+return 0;
+
+if (virTimeMillisNow(&now) < 0)
+return -1;
+
+jobInfo->timeElapsed = now - jobInfo->started;
+return 0;
+}
+
+int
+qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
+virDomainJobInfoPtr info)
+{
+info->timeElapsed = jobInfo->timeElapsed;
+info->timeRemaining = jobInfo->timeRemaining;
+
+info->memTotal = jobInfo->status.ram_total;
+info->memRemaining = jobInfo->status.ram_remaining;
+info->memProcessed = jobInfo->status.ram_transferred;
+
+info->fileTotal = jobInfo->status.disk_total;
+info->fileRemaining = jobInfo->status.disk_remaining;
+info->fileProcessed = jobInfo->status.disk_transferred;
+
+info->dataTotal = info->memTotal + info->fileTotal;
+info->dataRemaining = info->memRemaining + info->fileRemaining;
+info->dataProcessed = info->memProcessed + info->fileProcessed;
+
+return 0;
+}
+
+int
+qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
+  int *type,
+  virTypedParameterPtr *params,
+  int *nparams)
+{
+qemuMonitorMigrationStatus *status = &jobInfo->status;
+virTypedParameterPtr par = NULL;
+int maxpar = 0;
+int npar = 0;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_TIME_ELAPSED,
+jobInfo->timeElapsed) < 0)
+goto error;
+
+if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED &&
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_TIME_REMAINING,
+jobInfo->timeRemaining) < 0)
+goto error;
+
+if (status->downtime_set &&
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DOWNTIME,
+status->downtime) < 0)
+goto error;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_TOTAL,
+status->ram_total +
+status->disk_total) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_PROCESSED,
+status->ram_transferred +
+status->disk_transferred) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_DATA_REMAINING,
+status->ram_remaining +
+status->disk_remaining) < 0)
+goto error;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_TOTAL,
+status->ram_total) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+status->ram_transferred) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_REMAINING,
+status->ram_remaining) < 0)
+goto error;
+
+if (status->ram_duplicate_set) {
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_CONSTANT,
+status->r

[libvirt] [PATCH 2/6] Add support for fetching statistics of completed jobs

[Jiri Denemark]

virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that
can be used to fetch statistics of a completed job rather than a
currently running job.

Signed-off-by: Jiri Denemark 
---
 include/libvirt/libvirt.h.in | 11 +++
 src/libvirt.c|  8 +++-
 src/qemu/qemu_domain.c   |  1 +
 src/qemu/qemu_domain.h   |  1 +
 src/qemu/qemu_driver.c   | 25 +++--
 src/qemu/qemu_migration.c|  6 ++
 src/qemu/qemu_monitor_json.c |  3 ++-
 7 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 9358314..9670e06 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4306,6 +4306,17 @@ struct _virDomainJobInfo {
 unsigned long long fileRemaining;
 };
 
+/**
+ * virDomainGetJobStatsFlags:
+ *
+ * Flags OR'ed together to provide specific behavior when querying domain
+ * job statistics.
+ */
+typedef enum {
+VIR_DOMAIN_JOB_STATS_COMPLETED = 1 << 0, /* return stats of a recently
+  * completed job */
+} virDomainGetJobStatsFlags;
+
 int virDomainGetJobInfo(virDomainPtr dom,
 virDomainJobInfoPtr info);
 int virDomainGetJobStats(virDomainPtr domain,
diff --git a/src/libvirt.c b/src/libvirt.c
index 5d8f01c..6fa0a6b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17567,7 +17567,7 @@ virDomainGetJobInfo(virDomainPtr domain, 
virDomainJobInfoPtr info)
  * @type: where to store the job type (one of virDomainJobType)
  * @params: where to store job statistics
  * @nparams: number of items in @params
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainGetJobStatsFlags
  *
  * Extract information about progress of a background job on a domain.
  * Will return an error if the domain is not active. The function returns
@@ -17577,6 +17577,12 @@ virDomainGetJobInfo(virDomainPtr domain, 
virDomainJobInfoPtr info)
  * may receive fields that they do not understand in case they talk to a
  * newer server.
  *
+ * When @flags contains VIR_DOMAIN_JOB_STATS_COMPLETED, the function will
+ * return statistics about a recently completed job. Specifically, this
+ * flag may be used to query statistics of a completed incoming migration.
+ * Statistics of a completed job are automatically destroyed once read or
+ * when libvirtd is restarted.
+ *
  * Returns 0 in case of success and -1 in case of failure.
  */
 int
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2c709e9..18a3761 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -199,6 +199,7 @@ static void
 qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
 {
 VIR_FREE(priv->job.current);
+VIR_FREE(priv->job.completed);
 virCondDestroy(&priv->job.cond);
 virCondDestroy(&priv->job.asyncCond);
 }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 119590e..365238b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -124,6 +124,7 @@ struct qemuDomainJobObj {
 unsigned long long mask;/* Jobs allowed during async job */
 bool dump_memory_only;  /* use dump-guest-memory to do dump */
 qemuDomainJobInfoPtr current;   /* async job progress data */
+qemuDomainJobInfoPtr completed; /* statistics data of a recently 
completed job */
 bool asyncAbort;/* abort of async job requested */
 };
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 265315d..cd6932a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11651,9 +11651,10 @@ qemuDomainGetJobStats(virDomainPtr dom,
 {
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv;
+qemuDomainJobInfoPtr jobInfo;
 int ret = -1;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1);
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
@@ -11663,13 +11664,19 @@ qemuDomainGetJobStats(virDomainPtr dom,
 if (virDomainGetJobStatsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
+if (!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) &&
+!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
 goto cleanup;
 }
 
-if (!priv->job.current) {
+if (flags & VIR_DOMAIN_JOB_STATS_COMPLETED)
+jobInfo = priv->job.completed;
+else
+jobInfo = priv->job.current;
+
+if (!jobInfo) {
 *type = VIR_DOMAIN_JOB_NONE;
 *params = NULL;
 *nparams = 0;
@@ -11682,11 +11689,17 @@ qemuDomainGetJobStats(virDomainPtr dom,
  * of incoming migration which we don't currently
  * monitor actively in the background thread
  */
-if (qemuDomainJobInfoUpdateTime(priv->job.current) < 0 ||
-qemuDomainJobInfoToParams(priv->job.current,
- 

[libvirt] [PATCH 0/6] Add support for fetching statistics of completed jobs

[Jiri Denemark]

Using virDomainGetJobStats, we can monitor running jobs but sometimes it
may be useful to get statistics about a job that already finished, for
example, to get the final amount of data transferred during migration or
to get an idea about total downtime. This is what the following patches
are about.

Jiri Denemark (6):
  Refactor job statistics
  Add support for fetching statistics of completed jobs
  virsh: Add support for completed job stats
  qemu: Transfer migration statistics to destination
  qemu: Recompute downtime and total time when migration completes
  qemu: Transfer recomputed stats back to source

 include/libvirt/libvirt.h.in |  11 ++
 src/libvirt.c|  11 +-
 src/qemu/qemu_domain.c   | 186 +-
 src/qemu/qemu_domain.h   |  27 +++-
 src/qemu/qemu_driver.c   | 130 --
 src/qemu/qemu_migration.c| 304 ---
 src/qemu/qemu_monitor_json.c |   3 +-
 src/qemu/qemu_process.c  |   9 +-
 tools/virsh-domain.c |  27 +++-
 tools/virsh.pod  |   8 +-
 10 files changed, 545 insertions(+), 171 deletions(-)

-- 
2.1.0

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

[libvirt] [PATCH v6 0/3] OVMF exposure

[Michal Privoznik]

Diff to v5:
- changed readonly='on|off' to readonly='yes|no'
- rebased to current upstream

Michal Privoznik (3):
  conf: Extend  and introduce 
  qemu: Implement extended loader and nvram
  qemu: Automatically create NVRAM store

 docs/formatdomain.html.in  |  22 +++-
 docs/schemas/domaincommon.rng  |  28 +
 libvirt.spec.in|   2 +
 src/Makefile.am|   1 +
 src/conf/domain_conf.c |  96 ++-
 src/conf/domain_conf.h |  23 +++-
 src/libvirt_private.syms   |   3 +
 src/qemu/libvirtd_qemu.aug |   3 +
 src/qemu/qemu.conf |  14 +++
 src/qemu/qemu_command.c|  97 ++-
 src/qemu/qemu_conf.c   |  94 ++
 src/qemu/qemu_conf.h   |   5 +
 src/qemu/qemu_process.c| 137 +
 src/qemu/test_libvirtd_qemu.aug.in |   3 +
 src/security/security_dac.c|   8 ++
 src/security/security_selinux.c|   8 ++
 src/security/virt-aa-helper.c  |   4 +-
 src/vbox/vbox_common.c |   7 +-
 src/xenapi/xenapi_driver.c |   3 +-
 src/xenconfig/xen_common.c |   7 +-
 src/xenconfig/xen_sxpr.c   |  16 +--
 tests/domainschemadata/domain-bios-nvram-empty.xml |  40 ++
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  |  10 ++
 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml |  40 ++
 tests/qemuxml2argvtest.c   |   2 +
 .../qemuxml2xmlout-pci-bridge-many-disks.xml   |   2 +-
 tests/qemuxml2xmltest.c|   2 +
 tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml  |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml  |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml  |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml  |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml  |   2 +-
 .../sexpr2xml-fv-serial-dev-2-ports.xml|   2 +-
 .../sexpr2xml-fv-serial-dev-2nd-port.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml  |   2 +-
 .../sexpr2xml-fv-serial-tcp-telnet.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-sound.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml  |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-utc.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-v2.xml|   2 +-
 tests/sexpr2xmldata/sexpr2xml-fv.xml   |   2 +-
 tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml  |   2 +-
 tests/xmconfigdata/test-escape-paths.xml   |   2 +-
 tests/xmconfigdata/test-fullvirt-force-hpet.xml|   2 +-
 tests/xmconfigdata/test-fullvirt-force-nohpet.xml  |   2 +-
 tests/xmconfigdata/test-fullvirt-localtime.xml |   2 +-
 tests/xmconfigdata/test-fullvirt-net-ioemu.xml |   2 +-
 tests/xmconfigdata/test-fullvirt-net-netfront.xml  |   2 +-
 tests/xmconfigdata/test-fullvirt-new-cdrom.xml |   2 +-
 tests/xmconfigdata/test-fullvirt-old-cdrom.xml |   2 +-
 tests/xmconfigdata/test-fullvirt-parallel-tcp.xml  |   2 +-
 .../test-fullvirt-serial-dev-2-ports.xml   |   2 +-
 .../test-fullvirt-serial-dev-2nd-port.xml  |   2 +-
 tests/xmconfigdata/test-fullvirt-serial-file.xml   |   2 +-
 tests/xmconfigdata/test-fullvirt-serial-null.xml   |   2 +-
 tests/xmconfigdata/test-fullvirt-serial-pipe.xml   |   2 +-
 tests/xmconfigdata/test-fullvirt-serial-pty.xml|   2 +-
 tests/xmconfigdata/test-fullvirt-serial-stdio.xml  |   2 +-
 .../test-fullvirt-serial-tcp-telnet.xml|   2 +-
 tests/xmconfigdata/test-fullvirt-serial-tcp.xml|   2 +-
 tests/xmconfigdata/test-fullvirt-serial-udp.xml|   2 +-
 tests/xmconfigdata/test-fullvirt-serial-unix.xml   |   2 +-
 tests/xmconfigdata/test-fullvirt-sound.xml |   2 +-
 test

[libvirt] [PATCH v6 2/3] qemu: Implement extended loader and nvram

[Michal Privoznik]

QEMU now supports UEFI with the following command line:

  -drive 
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \

where the first line reflects  and the second one .
Moreover, these two lines obsoletes the -bios argument.

Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.

Signed-off-by: Michal Privoznik 
Acked-by: Laszlo Ersek 
---
 src/qemu/qemu_command.c| 94 +-
 src/security/security_dac.c|  8 ++
 src/security/security_selinux.c|  8 ++
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  | 10 +++
 tests/qemuxml2argvtest.c   |  2 +
 5 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3cb2e0b..510f378 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7370,6 +7370,94 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd,
 return 0;
 }
 
+static int
+qemuBuilDomainLoaderCommandLine(virCommandPtr cmd,
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps)
+{
+int ret = -1;
+virDomainLoaderDefPtr loader = def->os.loader;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int unit = 0;
+
+if (!loader)
+return 0;
+
+switch ((virDomainLoader) loader->type) {
+case VIR_DOMAIN_LOADER_TYPE_ROM:
+virCommandAddArg(cmd, "-bios");
+virCommandAddArg(cmd, loader->path);
+break;
+
+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+/* UEFI is supported only for x86_64 currently */
+if (def->os.arch != VIR_ARCH_X86_64) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("pflash is not supported for %s guest 
achitecture"),
+   virArchToString(def->os.arch));
+goto cleanup;
+}
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support -drive"));
+goto cleanup;
+}
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support passing "
+ "drive format"));
+goto cleanup;
+}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI) &&
+def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("ACPI must be enabled in order to use UEFI"));
+goto cleanup;
+}
+
+virBufferAsprintf(&buf,
+  "file=%s,if=pflash,format=raw,unit=%d",
+  loader->path, unit);
+unit++;
+
+if (loader->readonly) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support passing "
+ "readonly attribute"));
+goto cleanup;
+}
+
+virBufferAsprintf(&buf, ",readonly=%s",
+  virTristateSwitchTypeToString(loader->readonly));
+}
+
+virCommandAddArg(cmd, "-drive");
+virCommandAddArgBuffer(cmd, &buf);
+
+if (loader->nvram) {
+virBufferFreeAndReset(&buf);
+virBufferAsprintf(&buf,
+  "file=%s,if=pflash,format=raw,unit=%d",
+  loader->nvram, unit);
+
+virCommandAddArg(cmd, "-drive");
+virCommandAddArgBuffer(cmd, &buf);
+}
+break;
+
+case VIR_DOMAIN_LOADER_TYPE_LAST:
+/* nada */
+break;
+}
+
+ret = 0;
+ cleanup:
+virBufferFreeAndReset(&buf);
+return ret;
+}
+
 qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
 .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
 };
@@ -7525,10 +7613,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, "-enable-nesting");
 }
 
-if (def->os.loader) {
-virCommandAddArg(cmd, "-bios");
-virCommandAddArg(cmd, def->os.loader->path);
-}
+if (qemuBuilDomainLoaderCommandLine(cmd, def, qemuCaps) < 0)
+goto error;
 
 /* Set '-m MB' based on maxmem, because the lower 'memory' limit
  * is set post-startup using the balloon driver. If balloon driver
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index e62828e..e

[libvirt] [PATCH v6 1/3] conf: Extend and introduce

[Michal Privoznik]

Up to now, users can configure BIOS via the  element. With
the upcoming implementation of UEFI this is not enough as BIOS and
UEFI are conceptually different. For instance, while BIOS is ROM, UEFI
is programmable flash (although all writes to code section are
denied). Therefore we need new attribute @type which will
differentiate the two. Then, new attribute @readonly is introduced to
reflect the fact that some images are RO.

Moreover, the OVMF (which is going to be used mostly), works in two
modes:
1) Code and UEFI variable store is mixed in one file.
2) Code and UEFI variable store is separated in two files

The latter has advantage of updating the UEFI code without losing the
configuration. However, in order to represent the latter case we need
yet another XML element: . Currently, it has no additional
attributes, it's just a bare element containing path to the variable
store file.

Signed-off-by: Michal Privoznik 
Acked-by: Laszlo Ersek 
---
 docs/formatdomain.html.in  | 19 -
 docs/schemas/domaincommon.rng  | 21 ++
 src/conf/domain_conf.c | 87 +-
 src/conf/domain_conf.h | 22 +-
 src/libvirt_private.syms   |  3 +
 src/qemu/qemu_command.c|  5 +-
 src/security/virt-aa-helper.c  |  4 +-
 src/vbox/vbox_common.c |  7 +-
 src/xenapi/xenapi_driver.c |  3 +-
 src/xenconfig/xen_common.c |  7 +-
 src/xenconfig/xen_sxpr.c   | 16 ++--
 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml | 40 ++
 .../qemuxml2xmlout-pci-bridge-many-disks.xml   |  2 +-
 tests/qemuxml2xmltest.c|  2 +
 tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml  |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml  |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml  |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml  |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml  |  2 +-
 .../sexpr2xml-fv-serial-dev-2-ports.xml|  2 +-
 .../sexpr2xml-fv-serial-dev-2nd-port.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml  |  2 +-
 .../sexpr2xml-fv-serial-tcp-telnet.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-sound.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml  |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-utc.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv-v2.xml|  2 +-
 tests/sexpr2xmldata/sexpr2xml-fv.xml   |  2 +-
 tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml  |  2 +-
 tests/xmconfigdata/test-escape-paths.xml   |  2 +-
 tests/xmconfigdata/test-fullvirt-force-hpet.xml|  2 +-
 tests/xmconfigdata/test-fullvirt-force-nohpet.xml  |  2 +-
 tests/xmconfigdata/test-fullvirt-localtime.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-net-ioemu.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-net-netfront.xml  |  2 +-
 tests/xmconfigdata/test-fullvirt-new-cdrom.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-old-cdrom.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-parallel-tcp.xml  |  2 +-
 .../test-fullvirt-serial-dev-2-ports.xml   |  2 +-
 .../test-fullvirt-serial-dev-2nd-port.xml  |  2 +-
 tests/xmconfigdata/test-fullvirt-serial-file.xml   |  2 +-
 tests/xmconfigdata/test-fullvirt-serial-null.xml   |  2 +-
 tests/xmconfigdata/test-fullvirt-serial-pipe.xml   |  2 +-
 tests/xmconfigdata/test-fullvirt-serial-pty.xml|  2 +-
 tests/xmconfigdata/test-fullvirt-serial-stdio.xml  |  2 +-
 .../test-fullvirt-serial-tcp-telnet.xml|  2 +-
 tests/xmconfigdata/test-fullvirt-serial-tcp.xml|  2 +-
 tests/xmconfigdata/test-fullvirt-serial-udp.xml|  2 +-
 tests/xmconfigdata/test-fullvirt-serial-unix.xml   |  2 +-
 tests/xmconfigdata/test-fullvirt-sound.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-usbmouse.xml  |  2 +-
 tests/xmconfigdata/test-fullvirt-usbtablet.xml |  2 +-
 tests/xmconfigdata/test-fullvirt-utc.x

[libvirt] [PATCH v6 3/3] qemu: Automatically create NVRAM store

[Michal Privoznik]

When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.

Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
 element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.

Signed-off-by: Michal Privoznik 
Acked-by: Laszlo Ersek 
---
 docs/formatdomain.html.in  |  11 +-
 docs/schemas/domaincommon.rng  |   9 +-
 libvirt.spec.in|   2 +
 src/Makefile.am|   1 +
 src/conf/domain_conf.c |  11 +-
 src/conf/domain_conf.h |   1 +
 src/qemu/libvirtd_qemu.aug |   3 +
 src/qemu/qemu.conf |  14 +++
 src/qemu/qemu_conf.c   |  94 ++
 src/qemu/qemu_conf.h   |   5 +
 src/qemu/qemu_process.c| 137 +
 src/qemu/test_libvirtd_qemu.aug.in |   3 +
 tests/domainschemadata/domain-bios-nvram-empty.xml |  40 ++
 13 files changed, 325 insertions(+), 6 deletions(-)
 create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 757035a..a2ea758 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -103,7 +103,7 @@
   
 hvm
 /usr/lib/xen/boot/hvmloader
-/var/lib/libvirt/nvram/guest_VARS.fd
+/var/lib/libvirt/nvram/guest_VARS.fd
 
 
 
@@ -142,9 +142,12 @@
 pflash.
   nvram
   Some UEFI firmwares may want to use a non-volatile memory to store
-some variables. In the host, this is represented as a file and the
-path to the file is stored in this element. Since
-1.2.8
+some variables. In the host, this is represented as a file and the path
+to the file is stored in this element. Moreover, when the domain is
+started up libvirt copies so called master NVRAM store file defined
+in qemu.conf. If needed, the template
+attribute can be used to per domain override map of master NVRAM stores
+from the config file. Since 1.2.8
   boot
   The dev attribute takes one of the values "fd", "hd",
 "cdrom" or "network" and is used to specify the next boot device
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5d9c21c..6ae940a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -263,7 +263,14 @@
 
 
   
-
+
+  
+
+  
+
+
+  
+
   
 
 
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b7a26a1..5da8940 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1942,6 +1942,7 @@ exit 0
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/channel/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/channel/target/
+%dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/nvram/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/cache/libvirt/qemu/
 %{_datadir}/augeas/lenses/libvirtd_qemu.aug
 %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
@@ -2044,6 +2045,7 @@ exit 0
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/channel/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/channel/target/
+%dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/lib/libvirt/qemu/nvram/
 %dir %attr(0750, %{qemu_user}, %{qemu_group}) 
%{_localstatedir}/cache/libvirt/qemu/
 %{_datadir}/augeas/lenses/libvirtd_qemu.aug
 %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
diff --git a/src/Makefile.am b/src/Makefile.am
index 46e411e..fa741a8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2679,6 +2679,7 @@ endif WITH_SANLOCK
 if WITH_QEMU
$(MKDIR_P) "$(DESTDIR

Re: [libvirt] [PATCH] virsh: Implement command to rename domain

[Tomas Meszaros]

On 01/09/14 at 10:31am, Philipp Hahn wrote:
> This has been requested in the past already (even by me ;-)
> Renaming is not that simple, as there are several more things to do:
> 1. Rename log files (this was somehow controversial last time it was
> discussed, especially combined with external programs like logrotate)
> 2. Fix domain config for suspended VMs.
> 3. Keep existing snapshots
> 3.1 Fix domain config in snapshots.
> 
> Especially the last thing does very bad things if you revert a renamed
> VM, as the UUID is then no longer unique.
> 
> Philipp "sorry, no patch" Hahn

Oh... so I will have to stick with the old way...
Anyway, thank you for clarification Philipp.


Tomas

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

[libvirt] [python PATCH v2 2/5] API: Skip 'virDomainStatsRecordListFree'

[Pavel Hrdina]

From: Peter Krempa 

The new API function doesn't make sense to be exported in python. The
bindings will return native types instead of the struct array.

Signed-off-by: Peter Krempa 
Signed-off-by: Pavel Hrdina 
---
 generator.py  | 1 +
 sanitytest.py | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/generator.py b/generator.py
index 0d41e20..c4c12df 100755
--- a/generator.py
+++ b/generator.py
@@ -571,6 +571,7 @@ skip_function = (
 "virTypedParamsGetULLong",
 
 'virNetworkDHCPLeaseFree', # only useful in C, python code uses list
+'virDomainStatsRecordListFree', # only useful in C, python uses dict
 )
 
 lxc_skip_function = (
diff --git a/sanitytest.py b/sanitytest.py
index 4f4a648..10cf9f0 100644
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -81,6 +81,9 @@ for cname in wantfunctions:
 if name[0:23] == "virNetworkDHCPLeaseFree":
 continue
 
+if name[0:28] == "virDomainStatsRecordListFree":
+continue
+
 # These aren't functions, they're callback signatures
 if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc",
 "virStreamSinkFunc", "virStreamSourceFunc", 
"virStreamEventCallback",
-- 
1.8.5.5

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

[libvirt] [python PATCH v2 0/5] Implement new APIs

[Pavel Hrdina]

new in v2:
- moved function to appropriate place in libvirt-override.c
- fixed generator to resolve enum reference
- fixed memory leak in virDomainListGetStats and sanyti test
- implemented API for virDomainBlockCopy

Pavel Hrdina (3):
  generator: resolve one level of enum reference
  API: Implement bindings for virDomainListGetStats
  Implement API bindings for virDomainBlockCopy

Peter Krempa (2):
  API: Skip 'virDomainStatsRecordListFree'
  API: Implement bindings for virConnectGetAllDomainStats

 generator.py   |  20 -
 libvirt-override-api.xml   |  10 +++
 libvirt-override-virConnect.py | 100 +++
 libvirt-override.c | 180 +
 sanitytest.py  |   6 ++
 5 files changed, 315 insertions(+), 1 deletion(-)

-- 
1.8.5.5

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

[libvirt] [python PATCH v2 3/5] API: Implement bindings for virConnectGetAllDomainStats

[Pavel Hrdina]

From: Peter Krempa 

Implement the function by returning a list of tuples instead the array
of virDomainStatsRecords and store the typed parameters as dict.

Signed-off-by: Peter Krempa 
Signed-off-by: Pavel Hrdina 
---
 generator.py   |  1 +
 libvirt-override-virConnect.py | 53 +++
 libvirt-override.c | 95 ++
 3 files changed, 149 insertions(+)

diff --git a/generator.py b/generator.py
index c4c12df..3fc7db2 100755
--- a/generator.py
+++ b/generator.py
@@ -507,6 +507,7 @@ skip_function = (
 'virConnectListAllNodeDevices', # overridden in virConnect.py
 'virConnectListAllNWFilters', # overridden in virConnect.py
 'virConnectListAllSecrets', # overridden in virConnect.py
+'virConnectGetAllDomainStats', # overridden in virConnect.py
 
 'virStreamRecvAll', # Pure python libvirt-override-virStream.py
 'virStreamSendAll', # Pure python libvirt-override-virStream.py
diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py
index 31d71a3..c4c400a 100644
--- a/libvirt-override-virConnect.py
+++ b/libvirt-override-virConnect.py
@@ -383,3 +383,56 @@
 if ret is None:raise libvirtError('virDomainCreateXMLWithFiles() 
failed', conn=self)
 __tmp = virDomain(self,_obj=ret)
 return __tmp
+
+def getAllDomainStats(self, stats = 0, flags=0):
+"""Query statistics for all domains on a given connection.
+
+Report statistics of various parameters for a running VM according to 
@stats
+field. The statistics are returned as an array of structures for each 
queried
+domain. The structure contains an array of typed parameters containing 
the
+individual statistics. The typed parameter name for each statistic 
field
+consists of a dot-separated string containing name of the requested 
group
+followed by a group specific description of the statistic value.
+
+The statistic groups are enabled using the @stats parameter which is a
+binary-OR of enum virDomainStatsTypes. The following groups are 
available
+(although not necessarily implemented for each hypervisor):
+
+VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering 
that
+state. The typed parameter keys are in this format:
+"state.state" - state of the VM, returned as int from virDomainState 
enum
+"state.reason" - reason for entering given state, returned as int from
+ virDomain*Reason enum corresponding to given state.
+
+Using 0 for @stats returns all stats groups supported by the given
+hypervisor.
+
+Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags 
makes
+the function return error in case some of the stat types in @stats were
+not recognized by the daemon.
+
+Similarly to virConnectListAllDomains, @flags can contain various 
flags to
+filter the list of domains to provide stats for.
+
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE selects online domains while
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE selects offline ones.
+
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT and
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_TRANSIENT allow to filter the list
+according to their persistence.
+
+To filter the list of VMs by domain state @flags can contain
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_RUNNING,
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED,
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states. """
+ret = libvirtmod.virConnectGetAllDomainStats(self._o, stats, flags)
+if ret is None:
+raise libvirtError("virConnectGetAllDomainStats() failed", 
conn=self)
+
+retlist = list()
+for elem in ret:
+record = (virDomain(self, _obj=elem[0]) , elem[1])
+retlist.append(record)
+
+return retlist
diff --git a/libvirt-override.c b/libvirt-override.c
index b2271ae..2da43ab 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7955,6 +7955,98 @@ libvirt_virNetworkGetDHCPLeases(PyObject *self 
ATTRIBUTE_UNUSED,
 
 #endif /* LIBVIR_CHECK_VERSION(1, 2, 6) */
 
+#if LIBVIR_CHECK_VERSION(1, 2, 8)
+
+static PyObject *
+convertDomainStatsRecord(virDomainStatsRecordPtr *records,
+ int nrecords)
+{
+PyObject *py_retval;
+PyObject *py_record;
+PyObject *py_record_domain;
+PyObject *py_record_stats;
+size_t i;
+
+if (!(py_retval = PyList_New(nrecords)))
+return NULL;
+
+for (i = 0; i < nrecords; i++) {
+if (!(py_record = PyTuple_New(2)))
+goto error;
+
+/* libvirt_virDomainPtrWrap steals the object */
+virDomainRef(records[i]->dom);
+if (!(py_record_domain = libvirt_virDomainPtrWrap(records[i]->dom))) {
+virDomainFr

[libvirt] [python PATCH v2 1/5] generator: resolve one level of enum reference

[Pavel Hrdina]

In the libvirt.h we have one enum defined by references from another
enum and it leads in wrong order of definitons in python code. To
prevent this we should resolve that references before we generate the
python code.

For now we have only one level of references so we will count with that
in the generator but we should update it in the future to be more
flexible.

Signed-off-by: Pavel Hrdina 
---
 generator.py | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/generator.py b/generator.py
index a12c52b..0d41e20 100755
--- a/generator.py
+++ b/generator.py
@@ -1785,12 +1785,26 @@ def buildWrappers(module):
 value = float('inf')
 return value
 
+# Resolve only one level of reference
+def resolveEnum(enum, data):
+for name,val in enum.items():
+try:
+int(val)
+except ValueError:
+enum[name] = data[val]
+return enum
+
 enumvals = list(enums.items())
+# convert list of dicts to one dict
+enumData = {}
+for type,enum in enumvals:
+enumData.update(enum)
+
 if enumvals is not None:
 enumvals.sort(key=lambda x: x[0])
 for type,enum in enumvals:
 classes.write("# %s\n" % type)
-items = list(enum.items())
+items = list(resolveEnum(enum, enumData).items())
 items.sort(key=enumsSortKey)
 if items[-1][0].endswith('_LAST'):
 del items[-1]
-- 
1.8.5.5

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

[libvirt] [python PATCH v2 4/5] API: Implement bindings for virDomainListGetStats

[Pavel Hrdina]

Implement the function by returning a list of tuples instead the array
of virDomainStatsRecords and store the typed parameters as dict.

Signed-off-by: Peter Krempa 
Signed-off-by: Pavel Hrdina 
---
 generator.py   |  1 +
 libvirt-override-virConnect.py | 47 ++
 libvirt-override.c | 52 ++
 sanitytest.py  |  3 +++
 4 files changed, 103 insertions(+)

diff --git a/generator.py b/generator.py
index 3fc7db2..1daf866 100755
--- a/generator.py
+++ b/generator.py
@@ -508,6 +508,7 @@ skip_function = (
 'virConnectListAllNWFilters', # overridden in virConnect.py
 'virConnectListAllSecrets', # overridden in virConnect.py
 'virConnectGetAllDomainStats', # overridden in virConnect.py
+'virDomainListGetStats', # overriden in virConnect.py
 
 'virStreamRecvAll', # Pure python libvirt-override-virStream.py
 'virStreamSendAll', # Pure python libvirt-override-virStream.py
diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py
index c4c400a..218f266 100644
--- a/libvirt-override-virConnect.py
+++ b/libvirt-override-virConnect.py
@@ -436,3 +436,50 @@
 retlist.append(record)
 
 return retlist
+
+def domainListGetStats(self, doms, stats=0, flags=0):
+""" Query statistics for given domains.
+
+Report statistics of various parameters for a running VM according to 
@stats
+field. The statistics are returned as an array of structures for each 
queried
+domain. The structure contains an array of typed parameters containing 
the
+individual statistics. The typed parameter name for each statistic 
field
+consists of a dot-separated string containing name of the requested 
group
+followed by a group specific description of the statistic value.
+
+The statistic groups are enabled using the @stats parameter which is a
+binary-OR of enum virDomainStatsTypes. The following groups are 
available
+(although not necessarily implemented for each hypervisor):
+
+VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering 
that
+state. The typed parameter keys are in this format:
+"state.state" - state of the VM, returned as int from virDomainState 
enum
+"state.reason" - reason for entering given state, returned as int from
+ virDomain*Reason enum corresponding to given state.
+
+Using 0 for @stats returns all stats groups supported by the given
+hypervisor.
+
+Specifying VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS as @flags 
makes
+the function return error in case some of the stat types in @stats were
+not recognized by the daemon.
+
+Get statistics about domains provided as a list in @doms. @stats is
+a bit field selecting requested statistics types."""
+domlist = list()
+for dom in doms:
+if not isinstance(dom, virDomain):
+raise libvirtError("domain list contains non-domain elements", 
conn=self)
+
+domlist.append(dom._o)
+
+ret = libvirtmod.virDomainListGetStats(self._o, domlist, stats, flags)
+if ret is None:
+raise libvirtError("virDomainListGetStats() failed", conn=self)
+
+retlist = list()
+for elem in ret:
+record = (virDomain(self, _obj=elem[0]) , elem[1])
+retlist.append(record)
+
+return retlist
diff --git a/libvirt-override.c b/libvirt-override.c
index 2da43ab..569778d 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -8045,6 +8045,57 @@ libvirt_virConnectGetAllDomainStats(PyObject *self 
ATTRIBUTE_UNUSED,
 return py_retval;
 }
 
+
+static PyObject *
+libvirt_virDomainListGetStats(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_conn;
+PyObject *py_retval;
+PyObject *py_domlist;
+virConnectPtr conn;
+virDomainStatsRecordPtr *records = NULL;
+virDomainPtr *doms = NULL;
+int nrecords;
+int ndoms;
+size_t i;
+unsigned int flags;
+unsigned int stats;
+
+if (!PyArg_ParseTuple(args, (char *)"OOii:virDomainListGetStats",
+  &pyobj_conn, &py_domlist, &stats, &flags))
+return NULL;
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+if (PyList_Check(py_domlist)) {
+ndoms = PyList_Size(py_domlist);
+
+if (VIR_ALLOC_N(doms, ndoms + 1) < 0)
+return PyErr_NoMemory();
+
+for (i = 0; i < ndoms; i++)
+doms[i] = PyvirDomain_Get(PyList_GetItem(py_domlist, i));
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+nrecords = virDomainListGetStats(doms, stats, &records, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (nrecords < 0) {
+py_retval = VIR_PY_NONE;
+goto cleanup;
+}
+
+if (!(py_retval = convertDomain

[libvirt] [python PATCH v2 5/5] Implement API bindings for virDomainBlockCopy

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 generator.py |  1 +
 libvirt-override-api.xml | 10 ++
 libvirt-override.c   | 33 +
 3 files changed, 44 insertions(+)

diff --git a/generator.py b/generator.py
index 1daf866..a798274 100755
--- a/generator.py
+++ b/generator.py
@@ -464,6 +464,7 @@ skip_impl = (
 'virConnectGetCPUModelNames',
 'virNodeGetFreePages',
 'virNetworkGetDHCPLeases',
+'virDomainBlockCopy',
 )
 
 lxc_skip_impl = (
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
index 09bbbf8..f0049d7 100644
--- a/libvirt-override-api.xml
+++ b/libvirt-override-api.xml
@@ -640,5 +640,15 @@
   
   
 
+
+  Copy the guest-visible contents of a disk image to a new file 
described by destxml
+  
+  
+  
+  
+  
+  
+  
+
   
 
diff --git a/libvirt-override.c b/libvirt-override.c
index 569778d..733a16f 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -8096,6 +8096,38 @@ libvirt_virDomainListGetStats(PyObject *self 
ATTRIBUTE_UNUSED,
 return py_retval;
 }
 
+
+static PyObject *
+libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
+{
+PyObject *pyobj_dom = NULL;
+PyObject *pyobj_dict = NULL;
+
+virDomainPtr dom;
+char *disk = NULL;
+char *destxml = NULL;
+virTypedParameterPtr params;
+int nparams;
+unsigned int flags;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char *) "OzzOii:virDomainBlockCopy",
+  &pyobj_dom, &disk, &destxml, &pyobj_dict, &nparams,
+  &flags))
+return VIR_PY_INT_FAIL;
+
+if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams, NULL, 0) < 0)
+return VIR_PY_INT_FAIL;
+
+dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainBlockCopy(dom, disk, destxml, params, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+return libvirt_intWrap(c_retval);
+}
+
 #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
 
 /
@@ -8286,6 +8318,7 @@ static PyMethodDef libvirtMethods[] = {
 #if LIBVIR_CHECK_VERSION(1, 2, 8)
 {(char *) "virConnectGetAllDomainStats", 
libvirt_virConnectGetAllDomainStats, METH_VARARGS, NULL},
 {(char *) "virDomainListGetStats", libvirt_virDomainListGetStats, 
METH_VARARGS, NULL},
+{(char *) "virDomainBlockCopy", libvirt_virDomainBlockCopy, METH_VARARGS, 
NULL},
 #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
 {NULL, NULL, 0, NULL}
 };
-- 
1.8.5.5

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

Re: [libvirt] [python PATCH v2 2/5] API: Skip 'virDomainStatsRecordListFree'

[Peter Krempa]

On 09/01/14 22:18, Pavel Hrdina wrote:
> From: Peter Krempa 
> 
> The new API function doesn't make sense to be exported in python. The
> bindings will return native types instead of the struct array.
> 
> Signed-off-by: Peter Krempa 
> Signed-off-by: Pavel Hrdina 
> ---
>  generator.py  | 1 +
>  sanitytest.py | 3 +++
>  2 files changed, 4 insertions(+)
> 

> diff --git a/sanitytest.py b/sanitytest.py
> index 4f4a648..10cf9f0 100644
> --- a/sanitytest.py
> +++ b/sanitytest.py
> @@ -81,6 +81,9 @@ for cname in wantfunctions:
>  if name[0:23] == "virNetworkDHCPLeaseFree":
>  continue
>  
> +if name[0:28] == "virDomainStatsRecordListFree":
> +continue
> +

Thanks for catching this. I didn't run the sanity test :/

>  # These aren't functions, they're callback signatures
>  if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc",
>  "virStreamSinkFunc", "virStreamSourceFunc", 
> "virStreamEventCallback",
> 

ACK,

Peter




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

Re: [libvirt] [python PATCH v2 3/5] API: Implement bindings for virConnectGetAllDomainStats

[Peter Krempa]

On 09/01/14 22:18, Pavel Hrdina wrote:
> From: Peter Krempa 
> 
> Implement the function by returning a list of tuples instead the array
> of virDomainStatsRecords and store the typed parameters as dict.
> 
> Signed-off-by: Peter Krempa 
> Signed-off-by: Pavel Hrdina 
> ---
>  generator.py   |  1 +
>  libvirt-override-virConnect.py | 53 +++
>  libvirt-override.c | 95 
> ++
>  3 files changed, 149 insertions(+)
> 

Well, looks like you didn't change this one and as you've ACKed it in v1
it should stand.

Peter




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

Re: [libvirt] [python PATCH v2 4/5] API: Implement bindings for virDomainListGetStats

[Peter Krempa]

On 09/01/14 22:18, Pavel Hrdina wrote:
> Implement the function by returning a list of tuples instead the array
> of virDomainStatsRecords and store the typed parameters as dict.
> 
> Signed-off-by: Peter Krempa 
> Signed-off-by: Pavel Hrdina 
> ---
>  generator.py   |  1 +
>  libvirt-override-virConnect.py | 47 ++
>  libvirt-override.c | 52 
> ++
>  sanitytest.py  |  3 +++
>  4 files changed, 103 insertions(+)

The changes look good to me, so if no one else will object

ACK

Peter




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

Re: [libvirt] [python PATCH v2 5/5] Implement API bindings for virDomainBlockCopy

[Peter Krempa]

On 09/01/14 22:18, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  generator.py |  1 +
>  libvirt-override-api.xml | 10 ++
>  libvirt-override.c   | 33 +
>  3 files changed, 44 insertions(+)
> 
> diff --git a/generator.py b/generator.py
> index 1daf866..a798274 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -464,6 +464,7 @@ skip_impl = (
>  'virConnectGetCPUModelNames',
>  'virNodeGetFreePages',
>  'virNetworkGetDHCPLeases',
> +'virDomainBlockCopy',
>  )
>  
>  lxc_skip_impl = (
> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
> index 09bbbf8..f0049d7 100644
> --- a/libvirt-override-api.xml
> +++ b/libvirt-override-api.xml
> @@ -640,5 +640,15 @@
>
>
>  
> +
> +  Copy the guest-visible contents of a disk image to a new file 
> described by destxml
> +  
> +  
> +  
> +  
> +  

Humm, the python generator doesn't have nice typed parameter support. 
This actually wouldn't be usable from python as this python method 
is generated:


def blockCopy(self, disk, destxml, params, nparams, flags=0):
"""Copy the guest-visible contents of a disk image to a new file 
described by destxml """
ret = libvirtmod.virDomainBlockCopy(self._o, disk, destxml, params, 
nparams, flags)
if ret == -1: raise libvirtError ('virDomainBlockCopy() failed', 
dom=self)
return ret


As there is no nice way to construct a typed parameter in python
we usually convert them to or from dicts. This will require you
to write the python impl too and ...

> +  
> +  
> +
>
>  
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 569778d..733a16f 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -8096,6 +8096,38 @@ libvirt_virDomainListGetStats(PyObject *self 
> ATTRIBUTE_UNUSED,
>  return py_retval;
>  }
>  
> +
> +static PyObject *
> +libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> +{
> +PyObject *pyobj_dom = NULL;
> +PyObject *pyobj_dict = NULL;
> +
> +virDomainPtr dom;
> +char *disk = NULL;
> +char *destxml = NULL;
> +virTypedParameterPtr params;
> +int nparams;
> +unsigned int flags;
> +int c_retval;
> +
> +if (!PyArg_ParseTuple(args, (char *) "OzzOii:virDomainBlockCopy",
> +  &pyobj_dom, &disk, &destxml, &pyobj_dict, &nparams,

... drop nparams here as that is infered from the length of the dict ...

> +  &flags))
> +return VIR_PY_INT_FAIL;
> +
> +if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams, NULL, 0) < 0)

and finally overwritten here.

> +return VIR_PY_INT_FAIL;
> +
> +dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
> +
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +c_retval = virDomainBlockCopy(dom, disk, destxml, params, nparams, 
> flags);
> +LIBVIRT_END_ALLOW_THREADS;
> +
> +return libvirt_intWrap(c_retval);
> +}
> +
>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>  
>  /
> @@ -8286,6 +8318,7 @@ static PyMethodDef libvirtMethods[] = {
>  #if LIBVIR_CHECK_VERSION(1, 2, 8)
>  {(char *) "virConnectGetAllDomainStats", 
> libvirt_virConnectGetAllDomainStats, METH_VARARGS, NULL},
>  {(char *) "virDomainListGetStats", libvirt_virDomainListGetStats, 
> METH_VARARGS, NULL},
> +{(char *) "virDomainBlockCopy", libvirt_virDomainBlockCopy, 
> METH_VARARGS, NULL},
>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>  {NULL, NULL, 0, NULL}
>  };
> 

You might be fine if you drop the "nparams" argument from the XML describing 
the API.
You also probably should make the argument optional by passing an empty dict or 
NONE
as a default value (IIRC by using keyword "Optional" in the description) 
depending on what 
virPyDictToTypedParams takes as missing typed params.

Peter



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

Re: [libvirt] [python PATCH v2 1/5] generator: resolve one level of enum reference

[Peter Krempa]

On 09/01/14 22:18, Pavel Hrdina wrote:
> In the libvirt.h we have one enum defined by references from another
> enum and it leads in wrong order of definitons in python code. To
> prevent this we should resolve that references before we generate the
> python code.
> 
> For now we have only one level of references so we will count with that
> in the generator but we should update it in the future to be more
> flexible.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  generator.py | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/generator.py b/generator.py
> index a12c52b..0d41e20 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -1785,12 +1785,26 @@ def buildWrappers(module):
>  value = float('inf')
>  return value
>  
> +# Resolve only one level of reference

We can make it recursive later if necessary.

> +def resolveEnum(enum, data):
> +for name,val in enum.items():
> +try:
> +int(val)
> +except ValueError:
> +enum[name] = data[val]
> +return enum
> +
>  enumvals = list(enums.items())
> +# convert list of dicts to one dict
> +enumData = {}
> +for type,enum in enumvals:
> +enumData.update(enum)

Update shouldn't ever update a key as it would trigger an error when
compiling the library code before we get here.

> +
>  if enumvals is not None:
>  enumvals.sort(key=lambda x: x[0])
>  for type,enum in enumvals:
>  classes.write("# %s\n" % type)
> -items = list(enum.items())
> +items = list(resolveEnum(enum, enumData).items())
>  items.sort(key=enumsSortKey)
>  if items[-1][0].endswith('_LAST'):
>  del items[-1]
> 

ACK

Peter



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

[libvirt] [python PATCH v3 5/5] Implement API bindings for virDomainBlockCopy

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---

new from v2:
- removed parameter nparams
- make params optional

 generator.py |  1 +
 libvirt-override-api.xml |  9 +
 libvirt-override.c   | 33 +
 3 files changed, 43 insertions(+)

diff --git a/generator.py b/generator.py
index 1daf866..a798274 100755
--- a/generator.py
+++ b/generator.py
@@ -464,6 +464,7 @@ skip_impl = (
 'virConnectGetCPUModelNames',
 'virNodeGetFreePages',
 'virNetworkGetDHCPLeases',
+'virDomainBlockCopy',
 )
 
 lxc_skip_impl = (
diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
index 09bbbf8..51d8273 100644
--- a/libvirt-override-api.xml
+++ b/libvirt-override-api.xml
@@ -640,5 +640,14 @@
   
   
 
+
+  Copy the guest-visible contents of a disk image to a new file 
described by destxml
+  
+  
+  
+  
+  
+  
+
   
 
diff --git a/libvirt-override.c b/libvirt-override.c
index 569778d..444a5fe 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -8096,6 +8096,38 @@ libvirt_virDomainListGetStats(PyObject *self 
ATTRIBUTE_UNUSED,
 return py_retval;
 }
 
+
+static PyObject *
+libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
+{
+PyObject *pyobj_dom = NULL;
+PyObject *pyobj_dict = NULL;
+
+virDomainPtr dom;
+char *disk = NULL;
+char *destxml = NULL;
+virTypedParameterPtr params;
+int nparams;
+unsigned int flags;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char *) "Ozz|Oi:virDomainBlockCopy",
+  &pyobj_dom, &disk, &destxml, &pyobj_dict, &nparams,
+  &flags))
+return VIR_PY_INT_FAIL;
+
+if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams, NULL, 0) < 0)
+return VIR_PY_INT_FAIL;
+
+dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainBlockCopy(dom, disk, destxml, params, nparams, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+return libvirt_intWrap(c_retval);
+}
+
 #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
 
 /
@@ -8286,6 +8318,7 @@ static PyMethodDef libvirtMethods[] = {
 #if LIBVIR_CHECK_VERSION(1, 2, 8)
 {(char *) "virConnectGetAllDomainStats", 
libvirt_virConnectGetAllDomainStats, METH_VARARGS, NULL},
 {(char *) "virDomainListGetStats", libvirt_virDomainListGetStats, 
METH_VARARGS, NULL},
+{(char *) "virDomainBlockCopy", libvirt_virDomainBlockCopy, METH_VARARGS, 
NULL},
 #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
 {NULL, NULL, 0, NULL}
 };
-- 
1.8.5.5

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

Re: [libvirt] [python PATCH v3 5/5] Implement API bindings for virDomainBlockCopy

[Peter Krempa]

On 09/02/14 00:08, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
> 
> new from v2:
> - removed parameter nparams
> - make params optional
> 
>  generator.py |  1 +
>  libvirt-override-api.xml |  9 +
>  libvirt-override.c   | 33 +
>  3 files changed, 43 insertions(+)
> 
> diff --git a/generator.py b/generator.py
> index 1daf866..a798274 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -464,6 +464,7 @@ skip_impl = (
>  'virConnectGetCPUModelNames',
>  'virNodeGetFreePages',
>  'virNetworkGetDHCPLeases',
> +'virDomainBlockCopy',
>  )
>  
>  lxc_skip_impl = (
> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
> index 09bbbf8..51d8273 100644
> --- a/libvirt-override-api.xml
> +++ b/libvirt-override-api.xml
> @@ -640,5 +640,14 @@
>
>
>  
> +
> +  Copy the guest-visible contents of a disk image to a new file 
> described by destxml
> +  
> +  
> +  
> +  
> +  
> +  
> +
>
>  
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 569778d..444a5fe 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -8096,6 +8096,38 @@ libvirt_virDomainListGetStats(PyObject *self 
> ATTRIBUTE_UNUSED,
>  return py_retval;
>  }
>  
> +
> +static PyObject *
> +libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> +{
> +PyObject *pyobj_dom = NULL;
> +PyObject *pyobj_dict = NULL;
> +
> +virDomainPtr dom;
> +char *disk = NULL;
> +char *destxml = NULL;
> +virTypedParameterPtr params;
> +int nparams;
> +unsigned int flags;
> +int c_retval;
> +
> +if (!PyArg_ParseTuple(args, (char *) "Ozz|Oi:virDomainBlockCopy",
> +  &pyobj_dom, &disk, &destxml, &pyobj_dict, &nparams,
> +  &flags))
> +return VIR_PY_INT_FAIL;

You need to wrap the call below into a if (PyDict_Check(pyobj_dict)) as
it doesn't handle "None" gracefully.

> +if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams, NULL, 0) < 0)
> +return VIR_PY_INT_FAIL;
> +
> +dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
> +
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +c_retval = virDomainBlockCopy(dom, disk, destxml, params, nparams, 
> flags);
> +LIBVIRT_END_ALLOW_THREADS;
> +
> +return libvirt_intWrap(c_retval);
> +}
> +
>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>  
>  /
> @@ -8286,6 +8318,7 @@ static PyMethodDef libvirtMethods[] = {
>  #if LIBVIR_CHECK_VERSION(1, 2, 8)
>  {(char *) "virConnectGetAllDomainStats", 
> libvirt_virConnectGetAllDomainStats, METH_VARARGS, NULL},
>  {(char *) "virDomainListGetStats", libvirt_virDomainListGetStats, 
> METH_VARARGS, NULL},
> +{(char *) "virDomainBlockCopy", libvirt_virDomainBlockCopy, 
> METH_VARARGS, NULL},
>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>  {NULL, NULL, 0, NULL}
>  };
> 

ACK with the comment above addressed.

Peter



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

Re: [libvirt] [python PATCH v3 5/5] Implement API bindings for virDomainBlockCopy

[Pavel Hrdina]

On 09/02/2014 12:17 AM, Peter Krempa wrote:
> On 09/02/14 00:08, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina 
>> ---
>>
>> new from v2:
>> - removed parameter nparams
>> - make params optional
>>
>>  generator.py |  1 +
>>  libvirt-override-api.xml |  9 +
>>  libvirt-override.c   | 33 +
>>  3 files changed, 43 insertions(+)
>>
>> diff --git a/generator.py b/generator.py
>> index 1daf866..a798274 100755
>> --- a/generator.py
>> +++ b/generator.py
>> @@ -464,6 +464,7 @@ skip_impl = (
>>  'virConnectGetCPUModelNames',
>>  'virNodeGetFreePages',
>>  'virNetworkGetDHCPLeases',
>> +'virDomainBlockCopy',
>>  )
>>  
>>  lxc_skip_impl = (
>> diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml
>> index 09bbbf8..51d8273 100644
>> --- a/libvirt-override-api.xml
>> +++ b/libvirt-override-api.xml
>> @@ -640,5 +640,14 @@
>>
>>
>>  
>> +
>> +  Copy the guest-visible contents of a disk image to a new file 
>> described by destxml
>> +  
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>>
>>  
>> diff --git a/libvirt-override.c b/libvirt-override.c
>> index 569778d..444a5fe 100644
>> --- a/libvirt-override.c
>> +++ b/libvirt-override.c
>> @@ -8096,6 +8096,38 @@ libvirt_virDomainListGetStats(PyObject *self 
>> ATTRIBUTE_UNUSED,
>>  return py_retval;
>>  }
>>  
>> +
>> +static PyObject *
>> +libvirt_virDomainBlockCopy(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
>> +{
>> +PyObject *pyobj_dom = NULL;
>> +PyObject *pyobj_dict = NULL;
>> +
>> +virDomainPtr dom;
>> +char *disk = NULL;
>> +char *destxml = NULL;
>> +virTypedParameterPtr params;
>> +int nparams;
>> +unsigned int flags;
>> +int c_retval;
>> +
>> +if (!PyArg_ParseTuple(args, (char *) "Ozz|Oi:virDomainBlockCopy",
>> +  &pyobj_dom, &disk, &destxml, &pyobj_dict, 
>> &nparams,
>> +  &flags))
>> +return VIR_PY_INT_FAIL;
> 
> You need to wrap the call below into a if (PyDict_Check(pyobj_dict)) as
> it doesn't handle "None" gracefully.
> 
>> +if (virPyDictToTypedParams(pyobj_dict, ¶ms, &nparams, NULL, 0) < 0)
>> +return VIR_PY_INT_FAIL;
>> +
>> +dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom);
>> +
>> +LIBVIRT_BEGIN_ALLOW_THREADS;
>> +c_retval = virDomainBlockCopy(dom, disk, destxml, params, nparams, 
>> flags);
>> +LIBVIRT_END_ALLOW_THREADS;
>> +
>> +return libvirt_intWrap(c_retval);
>> +}
>> +
>>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>>  
>>  /
>> @@ -8286,6 +8318,7 @@ static PyMethodDef libvirtMethods[] = {
>>  #if LIBVIR_CHECK_VERSION(1, 2, 8)
>>  {(char *) "virConnectGetAllDomainStats", 
>> libvirt_virConnectGetAllDomainStats, METH_VARARGS, NULL},
>>  {(char *) "virDomainListGetStats", libvirt_virDomainListGetStats, 
>> METH_VARARGS, NULL},
>> +{(char *) "virDomainBlockCopy", libvirt_virDomainBlockCopy, 
>> METH_VARARGS, NULL},
>>  #endif /* LIBVIR_CHECK_VERSION(1, 2, 8) */
>>  {NULL, NULL, 0, NULL}
>>  };
>>
> 
> ACK with the comment above addressed.
> 
> Peter
> 

Thanks for review, pushed whole series.

Pavel

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