Re: [libvirt] [PATCH v3 00/18] Implement virDomainBlockCopy
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
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
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
- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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'
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
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
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
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
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
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'
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
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
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
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
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
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
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
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