September 2015 - Devel - libvirt List Archives

[libvirt] Fwd: [PATCH v2] Close the source fd if the destination qemu exits during tunnelled migration
by Shivaprasad bhat 30 Sep '15

30 Sep '15

On Tue, Sep 29, 2015 at 2:33 AM, John Ferlan <jferlan(a)redhat.com> wrote: > > > On 09/28/2015 11:02 AM, Shivaprasad bhat wrote: >> Hi Jon, >> >> Thanks a lot for attaching the patch. Replies inline. >> >> On Thu, Sep 24, 2015 at 1:52 AM, John Ferlan <jferlan(a)redhat.com> wrote: >>> >>> >>> On 09/22/2015 07:21 AM, Shivaprasad bhat wrote: >>>> On Mon, Sep 21, 2015 at 8:04 PM, John Ferlan <jferlan(a)redhat.com> wrote: >>>>> >>>>> >>>>> On 09/21/2015 05:09 AM, Shivaprasad bhat wrote: >>>>>> Thanks John for the comments. >>>>>> >>>>>> >>>>>> On Fri, Sep 18, 2015 at 10:34 PM, John Ferlan <jferlan(a)redhat.com> wrote: >>>>>>> >>>>>>> >>>>>>> On 09/14/2015 10:44 AM, Shivaprasad G Bhat wrote: >>>>>>>> Tunnelled migration can hang if the destination qemu exits despite all the >>>>>>>> ABI checks. This happens whenever the destination qemu exits before the >>>>>>>> complete transfer is noticed by source qemu. The savevm state checks at >>>>>>>> runtime can fail at destination and cause qemu to error out. >>>>>>>> The source qemu cant notice it as the EPIPE is not propogated to it. >>>>>>>> The qemuMigrationIOFunc() notices the stream being broken from virStreamSend() >>>>>>>> and it cleans up the stream alone. The qemuMigrationWaitForCompletion() would >>>>>>>> never get to 100% transfer completion. >>>>>>>> The qemuMigrationWaitForCompletion() never breaks out as well since >>>>>>>> the ssh connection to destination is healthy, and the source qemu also thinks >>>>>>>> the migration is ongoing as the Fd to which it transfers, is never >>>>>>>> closed or broken. So, the migration will hang forever. Even Ctrl-C on the >>>>>>>> virsh migrate wouldn't be honoured. Close the source side FD when there is >>>>>>>> an error in the stream. That way, the source qemu updates itself and >>>>>>>> qemuMigrationWaitForCompletion() notices the failure. >>>>>>>> >>>>>>>> Close the FD for all kinds of errors to be sure. The error message is not >>>>>>>> copied for EPIPE so that the destination error is copied instead later. >>>>>>>> >>>>>>>> Note: >>>>>>>> Reproducible with repeated migrations between Power hosts running in different >>>>>>>> subcores-per-core modes. >>>>>>>> >>>>>>>> Changes from v1 -> v2: >>>>>>>> VIR_FORCE_CLOSE() was called twice for this use case which would log >>>>>>>> unneccessary warnings. So, move the fd close to qemuMigrationIOFunc >>>>>>>> so that there are no unnecessary duplicate attempts.(Would this trigger >>>>>>>> a Coverity error? I don't have a setup to check.) >>>>>>>> >>>>>>>> Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com> >>>>>>>> --- >>>>>>>> src/qemu/qemu_migration.c | 8 ++++++-- >>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >>>>>>>> index ff89ab5..9602fb2 100644 >>>>>>>> --- a/src/qemu/qemu_migration.c >>>>>>>> +++ b/src/qemu/qemu_migration.c >>>>>>>> @@ -4012,6 +4012,7 @@ static void qemuMigrationIOFunc(void *arg) >>>>>>>> if (virStreamFinish(data->st) < 0) >>>>>>>> goto error; >>>>>>>> >>>>>>>> + VIR_FORCE_CLOSE(data->sock); >>>>>>>> VIR_FREE(buffer); >>>>>>>> >>>>>>>> return; >>>>>>>> @@ -4029,7 +4030,11 @@ static void qemuMigrationIOFunc(void *arg) >>>>>>>> } >>>>>>>> >>>>>>>> error: >>>>>>>> - virCopyLastError(&data->err); >>>>>>>> + /* Let the source qemu know that the transfer cant continue anymore. >>>>>>>> + * Don't copy the error for EPIPE as destination has the actual error. */ >>>>>>>> + VIR_FORCE_CLOSE(data->sock); >>>>>>>> + if (!virLastErrorIsSystemErrno(EPIPE)) >>>>>>>> + virCopyLastError(&data->err); >>>>>>>> virResetLastError(); >>>>>>>> VIR_FREE(buffer); >>>>>>>> } >>>>>>>> @@ -4397,7 +4402,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, >>>>>>>> if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) >>>>>>>> ret = -1; >>>>>>>> } >>>>>>>> - VIR_FORCE_CLOSE(fd); >>>>>>> >>>>>>> ^^^ >>>>>>> >>>>>>> This causes Coverity to claim a RESOURCE_LEAK >>>>>>> >>>>>>> Feels like this was a mistake edit... >>>>>>> >>>>>> >>>>>> The VIR_FORCE_CLOSE() inside qemuMigrationIOFunc() alone is sufficient. >>>>>> Having this again here would lead to Warning in the logs. I too thought coverity >>>>>> would complain. Is there a way to force ignore a coverity warning? >>>>>> >>>>> >>>>> Typically a marker of sorts, such as >>>>> >>>>> /* coverity[leaked_handle] <some extra comment explaining why> */ >>>>> >>>>> Although I'm not sure that's the best way to handle this either. >>>>> >>>>> The problem I see though is this is an "error path" issue and when >>>>> perhaps it's safe/required to close fd/io->sock/data->sock. >>>>> >>>>> Your commit comment notes that the issue is seen on a fairly specific >>>>> event (virStreamSend failure) for a specific type of migration. >>>> >>>> I believe the failure can be seen for all types of migration with savestate >>>> mismatch. >>>> >>> >>> My thoughts were based mostly on your commit message comments: >>> >>> "The qemuMigrationIOFunc() notices the stream being broken from >>> virStreamSend() and it cleans up the stream alone. The >>> qemuMigrationWaitForCompletion() would never get to 100% transfer >>> completion." >>> >>> and the belief that the core issue you described is there's no way for >>> qemuMigrationCompleted to know the qemuMigrationIOFunc thread failed and >>> thus that's what caused the hang. Since we can pass the iothread "io" >>> object to the Completion function, I figured we could also use that to >>> check the status of the thread in the list of things already checked. I >>> guess it's not clear to me why your test fails below. Maybe it'll help >>> to attach/show you my thoughts in a patch format. I just took your >>> changes and "adjusted" them, merging the differences. So you could 'git >>> am' to top of trunk in order to compare my thoughts. >> >> I tried your patch. Gave migrations in a while loop. Hit the hang in 7th run >> itlsef. I feel the small time window is not very negligible. >> > > Perhaps I should clarify my query-migrate has no timeout comment... It > seems based on what I've read so far, the 'query-migrate' command > started successfully, because if it hadn't we would have received a > failure (as shown below). Thus libvirt has sent the command via the > monitor and is waiting for a response (e.g. the virCondWait after the > qemuMonitorSend in the trace below). The response isn't coming because > either "A" qemu didn't send it back or "B" libvirt missed it - that > should be determinable. > > There's a way to turn on debugging so the monitor dialog can be seen - > via changes to /etc/libvirt/libvirtd.conf. I use : > > log_level = 1 > log_filters="3:remote 4:event 3:json 3:rpc" > log_outputs="1:file:/var/log/libvirt/libvirtd.log" > > But you may need to remove the "3:json" in order to see the dialog since > that where it "feels like" the issue might be. Then start libvirtd in > the debugger again. Once it's hung - you should be able to scan (eg, > edit) the libvirtd.log file and search for the "query-migrate" command > being sent and then follow the copious output looking for the presence > of a returned command. If there is none, then something in qemu isn't > returning the failure correctly and it would need to be fixed there I > would think as opposed to throwing down the big hammer of closing the fd. Had a chance to run with your log settings. The query-migrate doesn't seem to have a corresponding "return" in the logs. So as you say, there may be a qemu bug that is not returning a response when the fd is still open(as libvirt didnt close it) but no read actually happening there. I felt qemu can't sense the failure as the fd is open, so posted this patch. Though, the qemu should return with the current state of migration as it sees instead of not returning at all. Hope we are on the same page. Mailer refused my reply as the attached logs with the mail crossed 300KB. Not attaching the logs for now. Please let me know if you want to me check anything specific. Thanks, Shivaprasad > > I think you've hit some edge/error "timing" condition - one of the more > difficult to debug. Finding the root cause of the problem could be > beneficial especially if it could be considered some more "common" type > problem for other similar migrate commands. > > John >>> >>> I'm not opposed to the chosen method, although perhaps Dan or Jiri who >>> understand more of the inner workings of migration than I do can provide >>> some comments. Changing where the fd is closed could have some corner >>> race condition I'm not aware of... >>> >>> One issue that needs to be resolved with your patch is what happens to >>> the 'fd' when qemuMigrationIOFunc isn't involved? IOW: if "fwdType == >>> MIGRATION_FWD_DIRECT", then qemuMigrationStartTunnel isn't called and >>> thus qemuMigrationIOFunc won't be closing the 'fd' in the form of >>> data->sock. That's perhaps a path Coverity was considering (and I hadn't >>> considered initially). >>> >>> I think to resolve this the following should be added to your patch (I >>> tried it, Coverity is happy too)... >>> >>> if (spec->fwdType != MIGRATION_FWD_DIRECT) { >>> if (!(iothread = qemuMigrationStartTunnel(spec->fwd.stream, fd))) >>> goto cancel; >>> /* If we've created a tunnel, then the 'fd' will be closed in the >>> * qemuMigrationIOFunc as data->sock >>> */ >>> fd = -1; >>> } >>> >>> and restore the VIR_FORCE_CLOSE(fd); >>> >> >> I agree. Let me know if we should go with this approach. I'll spin >> the next version with the above corrections. I am copying Jiri on this >> patch for his opinion as well. >> >>> >>>>> As I >>>>> read the code, that failure jumps to error (as does virStreamFinish). So >>>>> now you have a fairly specific set of instances which perhaps would >>>>> cause qemuMigrationWaitForCompletion to need to fail. The fix you have >>>>> proposed is to close the data->sock (io->sock, fd). However, your >>>>> proposal is a larger hammer. I assume closure of data->sock causes >>>>> WaitForCompletion to fail (perhaps differently) and that's why you chose it. >>>>> >>>>> Going back to the beginning of qemuMigrationRun, setting the 'fd' and >>>>> using StartTunnel seems to rely on MIGRATION_FWD_DIRECT, but if a tunnel >>>>> is created an (ugh) 'iothread' variable is NON-NULL. What if 'iothread' >>>>> were passed to qemuMigrationWaitForCompletion? Then again passed to >>>>> qemuMigrationCompleted which would check if iothread non-NULL and for >>>>> some new flag that could be created in _qemuMigrationIOThread, being >>>>> true. If it were true, then that would cause failure so that >>>>> WaitForCompletion would return error status. The only way the flag is >>>>> set to true is in qemuMigrationIOFunc when the code jumps to error. >>>>> (e.g. add bool stream_abort and data->stream_abort = true in "error:", >>>>> then check iothread && iothread->stream_abort, force error). >>>>> >>>> >>>> I tried this. Until the fd is closed, the 'info migrate' wouldn't >>>> return from the >>>> qemu. So, the migration hangs. Checking the stream status before/after >>>> fetching the job status would still leave a race. So, having it in a different >>>> thread (qemuMigrationIOFunc) here seems inevitable to me. >>> >>> Although perhaps susceptible to a smaller "timing" window, if we move >>> the iothread check to before the CheckJobStatus call in Completed, then >>> that'll work better? >>> >>> It seems the "query-migrate" command has no timeout... I assume qemu >>> would indicate that if you call it when the migration pipe had an error, >>> then you unpredictable results (only a guess - I didn't look at that >>> code as well). Perhaps that means libvirt needs some code to handle the >>> condition where the command doesn't finish in a timely manner. >>> >> >> No, If the pipe was broken, the query-migrate returns "failed". This is >> anyway handled in a generic way in libvirt. >> >> 90.408 > 0x3fff80007410 {"execute":"query-migrate","id":"libvirt-725"} >> 90.408 < 0x3fff80007410 {"return": {"status": "failed"}, "id": "libvirt-725"} >> >> Thanks and Regards, >> Shiva >> >>> Perhaps worth a shot... I did change my attached patch to reflect this >>> thought.. >>> >>> John >>>> >>>> #1 0x00003fff83ff593c in virCondWait (c=<optimized out>, m=<optimized out>) >>>> at util/virthread.c:154 >>>> #2 0x00003fff76235544 in qemuMonitorSend (mon=0x3fff54003970, >>>> msg=<optimized out>) at qemu/qemu_monitor.c:1035 >>>> #3 0x00003fff7624fb30 in qemuMonitorJSONCommandWithFd (mon=0x3fff54003970, >>>> cmd=0x3fff5c001420, scm_fd=-1, reply=0x3fff79fbd388) >>>> at qemu/qemu_monitor_json.c:293 >>>> #4 0x00003fff76254b90 in qemuMonitorJSONCommand (reply=0x3fff79fbd388, >>>> cmd=0x3fff5c001420, mon=0x3fff54003970) at qemu/qemu_monitor_json.c:323 >>>> #5 qemuMonitorJSONGetMigrationStatus (mon=0x3fff54003970, >>>> status=0x3fff79fbd538) at qemu/qemu_monitor_json.c:2620 >>>> #6 0x00003fff7623a664 in qemuMonitorGetMigrationStatus (mon=<optimized out>, >>>> status=<optimized out>) at qemu/qemu_monitor.c:2134 >>>> #7 0x00003fff76228e6c in qemuMigrationFetchJobStatus (driver=0x3fff6c118d80, >>>> vm=0x3fff6c27faf0, asyncJob=<optimized out>, jobInfo=0x3fff79fbd4f0) >>>> at qemu/qemu_migration.c:2528 >>>> #8 0x00003fff76228fb4 in qemuMigrationUpdateJobStatus (driver=0x3fff6c118d80, >>>> vm=0x3fff6c27faf0, asyncJob=QEMU_ASYNC_JOB_MIGRATION_OUT) >>>> at qemu/qemu_migration.c:2565 >>>> #9 0x00003fff76229200 in qemuMigrationCheckJobStatus (driver=0x3fff6c118d80, >>>> vm=0x3fff6c27faf0, asyncJob=QEMU_ASYNC_JOB_MIGRATION_OUT) >>>> ---Type <return> to continue, or q <return> to quit---cont >>>> at qemu/qemu_migration.c:2585 >>>> #10 0x00003fff76229408 in qemuMigrationCompleted (iothread=<optimized out>, >>>> >>>>> The only question then in my mind then is would this also be something >>>>> that should be done for the virStreamFinish failure path which also >>>>> jumps to error? >>>>> >>>> >>>> Yes, I too am not sure if its appropriate for virStreamFinish failure >>>> case. But, given >>>> this an error path, I feel we can safely go ahead and close the >>>> data->sock. >>>> >>>>> Doing this means you shouldn't need the VIR_FILE_CLOSE(data->sock) for >>>>> either the normal or error path. Does this seem to be a reasonable >>>>> approach and solve the issue you're facing? >>>>> >>>>> John >>>>>> Thanks and Regards, >>>>>> Shivaprasad >>>>>> >>>>>>> The rest of the patch looks reasonable; however, I'm far from the expert >>>>>>> in these matters. >>>>>>> >>>>>>> John >>>>>>>> >>>>>>>> if (priv->job.completed) { >>>>>>>> qemuDomainJobInfoUpdateTime(priv->job.completed); >>>>>>>> >>>>>>>> -- >>>>>>>> libvir-list mailing list >>>>>>>> libvir-list(a)redhat.com >>>>>>>> https://www.redhat.com/mailman/listinfo/libvir-list >>>>>>>>

1 0

[libvirt] [PATCH] testutils: Add coloring to verbose PASS/FAILED output
by Cole Robinson 30 Sep '15

30 Sep '15

Helps to visually track down test failures if debugging the test suite. The colors match what 'make check' does for pass/fail/skip --- tests/testutils.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 89026c6..bd4ff73 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -91,6 +91,11 @@ bool virtTestOOMActive(void) return testOOMActive; } +static int virtTestUseTerminalColors(void) +{ + return isatty(STDIN_FILENO); +} + static unsigned int virTestGetFlag(const char *name) { @@ -217,11 +222,20 @@ virtTestRun(const char *title, if (virTestGetVerbose()) { if (ret == 0) - fprintf(stderr, "OK\n"); + if (virtTestUseTerminalColors()) + fprintf(stderr, "\e[32mOK\e[0m\n"); /* green */ + else + fprintf(stderr, "OK\n"); else if (ret == EXIT_AM_SKIP) - fprintf(stderr, "SKIP\n"); + if (virtTestUseTerminalColors()) + fprintf(stderr, "\e[34m\e[1mSKIP\e[0m\n"); /* bold blue */ + else + fprintf(stderr, "SKIP\n"); else - fprintf(stderr, "FAILED\n"); + if (virtTestUseTerminalColors()) + fprintf(stderr, "\e[31m\e[1mFAILED\e[0m\n"); /* bold red */ + else + fprintf(stderr, "FAILED\n"); } else { if (testCounter != 1 && !((testCounter-1) % 40)) { -- 2.5.0

2 1

[libvirt] [PATCH 0/2] Check disk serial strings for uniqueness
by Peter Krempa 29 Sep '15

29 Sep '15

Peter Krempa (2): qemu: Perform the disk WWN check only on fresh starts conf: Reuse virDomainDefCheckDuplicateDiskWWN to check disk serial too src/conf/domain_conf.c | 15 ++++++++++++++- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_process.c | 3 ++- 4 files changed, 18 insertions(+), 4 deletions(-) -- 2.4.5

2 5

[libvirt] [PATCH tck 0/5] Getting the TCK to run against opensuse images
by Cédric Bosdonnat 29 Sep '15

29 Sep '15

Hi all, Here are a few patches to get the TCK run properly against opensuse image. The patch adding the opensuse images to virt-builder is still pending, but the main issues were: * Waiting for 30 seconds for the guest to be started is pretty fragile. For the tests pinging the guest, wait for domifstats to report 10 sent packets. * The first boot of the opensuse image runs dracut and takes quite a long time. We need to wait for the first boot to complete before shutting down the guest to avoid inconsistent states. * Yum isn't in all distros by default you know ;) Cédric Bosdonnat (5): Ping test: use domifstats to wait for dhcp to be setup in guest Create has_disk_image from code of get_virt_builder_disk Add DomainBuilder->rminterface function. Make sure the image has been started properly started the first time. 240-no-arp-spoofing.t: get virt-builder install dsniff lib/Sys/Virt/TCK.pm | 54 +++++++++++++++++++++++++++++-- lib/Sys/Virt/TCK/DomainBuilder.pm | 7 ++++ scripts/nwfilter/100-ping-still-working.t | 13 ++++++-- scripts/nwfilter/210-no-mac-spoofing.t | 13 ++++++-- scripts/nwfilter/230-no-mac-broadcast.t | 13 ++++++-- scripts/nwfilter/240-no-arp-spoofing.t | 14 ++++++-- 6 files changed, 103 insertions(+), 11 deletions(-) -- 2.1.4

3 7

[libvirt] [PATCH tck v2 0/5] Getting the TCK to run against opensuse images
by Cédric Bosdonnat 29 Sep '15

29 Sep '15

Hi all, Here are a few patches to get the TCK run properly against opensuse image. The patch adding the opensuse images to virt-builder is still pending, but the main issues were: * Waiting for 30 seconds for the guest to be started is pretty fragile. For the tests pinging the guest, wait for domifstats to report 10 sent packets. * The first boot of the opensuse image runs dracut and takes quite a long time. We need to wait for the first boot to complete before shutting down the guest to avoid inconsistent states. * Yum isn't in all distros by default you know ;) diff to v1: * set the proper email address Cédric Bosdonnat (5): Ping test: use domifstats to wait for dhcp to be setup in guest Create has_disk_image from code of get_virt_builder_disk Add DomainBuilder->rminterface function. Make sure the image has been started properly started the first time. 240-no-arp-spoofing.t: get virt-builder install dsniff lib/Sys/Virt/TCK.pm | 54 +++++++++++++++++++++++++++++-- lib/Sys/Virt/TCK/DomainBuilder.pm | 7 ++++ scripts/nwfilter/100-ping-still-working.t | 13 ++++++-- scripts/nwfilter/210-no-mac-spoofing.t | 13 ++++++-- scripts/nwfilter/230-no-mac-broadcast.t | 13 ++++++-- scripts/nwfilter/240-no-arp-spoofing.t | 14 ++++++-- 6 files changed, 103 insertions(+), 11 deletions(-) -- 2.1.4

1 5

[libvirt] [PATCH] qemu: Fix dynamic_ownership qemu.conf setting
by Cole Robinson 29 Sep '15

29 Sep '15

Commit 307fb904 (Sep 10) added a 'privileged' variable when creating the DAC driver: @@ -153,6 +157,7 @@ virSecurityManagerNewDAC(const char *virtDriver, bool defaultConfined, bool requireConfined, bool dynamicOwnership, + bool privileged, virSecurityManagerDACChownCallback chownCallback) But argument order is mixed up at the caller, swapping dynamicOwnership and privileged values. This corrects the argument order https://bugzilla.redhat.com/show_bug.cgi?id=1266628 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7025c51..aff1915 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -431,8 +431,8 @@ qemuSecurityInit(virQEMUDriverPtr driver) cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, cfg->securityRequireConfined, - virQEMUDriverIsPrivileged(driver), cfg->dynamicOwnership, + virQEMUDriverIsPrivileged(driver), qemuSecurityChownCallback))) goto error; if (!stack) { -- 2.5.0

2 2

[libvirt] [PATCH] conf: Fix virtType check
by John Ferlan 29 Sep '15

29 Sep '15

Commit id '7383b8cc' changed virDomainDef 'virtType' to an enum, that caused a build failure on some archs due to comparing an unsigned value to < 0. Adjust the fetch of 'type' to be into temporary 'int virtType' and then assign that virtType to the def->virtType Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- Pushed as build breaker src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cdafb14..393ece7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14640,7 +14640,7 @@ virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr *nodes = NULL, node = NULL; char *tmp = NULL; size_t i, j; - int n; + int n, virtType; long id = -1; virDomainDefPtr def; bool uuid_generated = false; @@ -14678,11 +14678,12 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - if ((def->virtType = virDomainVirtTypeFromString(tmp)) < 0) { + if ((virtType = virDomainVirtTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("invalid domain type %s"), tmp); goto error; } + def->virtType = virtType; VIR_FREE(tmp); def->os.bootloader = virXPathString("string(./bootloader)", ctxt); -- 2.1.0

2 1

[libvirt] [PATCH 0/4] improve virConnectListAllInterfaces()
by Laine Stump 29 Sep '15

29 Sep '15

There's a bit of background about this here: https://www.redhat.com/archives/augeas-devel/2015-September/msg00001.html In short, virt-manager is calling the virInterface APIs and that ties up a libvirt thread (and CPU core) for a very long time on hosts that have a large number of interfaces. These patches don't cure the problem (I don't know that there really is a cure other than "Don't DO that!"), but they do fix a couple of bugs I found while investigating, and make a substantial improvement in the amount of time used by virConnectListAllInterfaces(). One thing that I wondered about while investigating this - a big use of CPU by virConnectListAllInterfaces() comes from the need to retrieve the MAC address of every interface. The MAC addresses are both 1) returned to the caller in the interface objects and 2) sent to the policykit ACL checking to decide which interfaces to include in the list. I'm 90% confident that 1) most callers don't really care that they're getting the MAC address along with interface name (virt-manager, for example, follows up with a virInterfaceGetXMLDesc() anyway)), and 2) there is not even a single host *anywhere* that is using libvirt policykit ACLs to limit the list of host interfaces viewable by a client. So we could add a flag to not return MAC addresses, which would allow cutting down the time to list all devices to something < 1 second). But this would be at the expense of no longer having the possibility to limit the list with policykit according to MAC address. Since all host interface information is available to all users via the file system, for example, I don't see this as a huge issue, but it would change behavior, so I don't feel comfortable doing it. I don't like the idea of a single API call taking > 1 minute to return either, though. Does anyone have an opinion about this? Laine Stump (4): interface: fail on OOM from virGetInterface() interface: report correct interface count when not returning list interface: re-use name and mac address rather than re-retrieving interface: let netcf pre-filter for active vs. inactive src/interface/interface_backend_netcf.c | 56 +++++++++++++++------------------ 1 file changed, 25 insertions(+), 31 deletions(-) -- 2.4.3

4 15

[libvirt] [PATCH 0/2] Add support for gic-version machine option
by Pavel Fedin 29 Sep '15

29 Sep '15

qemu now supports gic-version option for the virt machine. This patch allows to use it in libvirt. I decided not to add a new capability because qemu correctly complains by itself if it does not have this option. Pavel Fedin (2): qemu: Add support for gic-version machine option qemu: Add test case for gic-version option src/qemu/qemu_command.c | 22 ++++++++---------- .../qemuxml2argv-aarch64-gicv3.args | 6 +++++ .../qemuxml2argv-aarch64-gicv3.xml | 26 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 43 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gicv3.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gicv3.xml -- 2.1.4

2 5

[libvirt] [libvirt-test-API] [PATCH] Add test cases for API defineXMLFlags
by Shanzhi Yu 29 Sep '15

29 Sep '15

Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- cases/linux_domain.conf | 22 +++++++++ repos/domain/definewithflags.py | 104 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 repos/domain/definewithflags.py diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index 6315613..7dd9062 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -214,6 +214,28 @@ domain:define macaddr 54:52:00:45:c3:8a +domain:undefine + guestname + $defaultname + +domain:definewithflags + guestname + $defaultname + diskpath + /var/lib/libvirt/images/libvirt-test-api + vcpu + 1 + memory + 1048576 + hddriver + virtio + nicdriver + virtio + macaddr + 54:52:00:45:c3:8a + flags + validate + domain:start guestname $defaultname diff --git a/repos/domain/definewithflags.py b/repos/domain/definewithflags.py new file mode 100644 index 0000000..01a4f15 --- /dev/null +++ b/repos/domain/definewithflags.py @@ -0,0 +1,104 @@ +#!/usr/bin/evn python + +import os +import re +import sys +import commands +import string +import pexpect + +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'diskpath', 'flags',) +optional_params = {'memory': 1048576, + 'vcpu': 1, + 'imageformat' : 'raw', + 'hddriver' : 'virtio', + 'nicdriver': 'virtio', + 'macaddr': '52:54:00:97:e4:28', + 'uuid' : '05867c1a-afeb-300e-e55e-2673391ae080', + 'username': None, + 'password': None, + 'virt_type': 'kvm', + 'xml': 'xmls/kvm_guest_define.xml', + 'guestarch': 'x86_64', + 'guestmachine': 'pc', + } + +def check_define_domain(guestname, virt_type, hostname, username, \ + password, logger): + """Check define domain result, if define domain is successful, + guestname.xml will exist under /etc/libvirt/qemu/ + and can use virt-xml-validate tool to check the file validity + """ + if "kvm" in virt_type: + path = "/etc/libvirt/qemu/%s.xml" % guestname + elif "xen" in virt_type: + path = "/etc/xen/%s" % guestname + else: + logger.error("unknown virt type") + + if hostname: + cmd = "ls %s" % path + ret, output = utils.remote_exec_pexpect(hostname, username, \ + password, cmd) + if ret: + logger.error("guest %s xml file doesn't exsits" % guestname) + return False + else: + return True + else: + if os.access(path, os.R_OK): + return True + else: + return False + +def definewithflags(params): + """Define a domain from xml""" + logger = params['logger'] + guestname = params['guestname'] + + xmlstr = params['xml'] + logger.debug("domain xml:\n%s" % xmlstr) + + conn = sharedmod.libvirtobj['conn'] + uri = conn.getURI() + + hostname = utils.parse_uri(uri)[1] + username = params.get('username', '') + password = params.get('password', '') + virt_type = params.get('virt_type', 'kvm') + + flags = params['flags'] + logger.info("The flags are %s" % flags) + flags_string = flags.split("|") + flags = 0 + + logger.info("define domain on %s" % uri) + + for flag in flags_string: + if flag == 'validate': + flags |= libvirt.VIR_DOMAIN_DEFINE_VALIDATE + else: + logger.error("unknow flags") + return 1 + + # Define domain from xml + try: + conn.defineXMLFlags(xmlstr, flags) + if check_define_domain(guestname, virt_type, hostname, \ + username, password, logger): + logger.info("define a domain from xml is successful") + else: + logger.error("fail to check define domain") + return 1 + except libvirtError, e: + logger.error("API error message: %s, error code is %s" \ + % (e.message, e.get_error_code())) + return 1 + + return 0 -- 1.8.3.1

1 0