August 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
by Osier Yang 19 Aug '11

19 Aug '11

* src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as "balloon: actual=", which cause "actual=" is stripped early before the real parsing. This patch changes BALLOON_PREFIX into "balloon: ", and modifies related functions, also renames "qemuMonitorParseExtraBalloonInfo" to "qemuMonitorParseBalloonInfo", as after the changing, it parses all the info returned by "info balloon". --- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++---------------- 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } - /* Convert bytes to kilobytes for libvirt */ switch (tag) { + /* Convert megabytes to kilobytes for libvirt */ + case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: + value = value << 10; + break; + /* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,<tag>=<val>]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, - virDomainMemoryStatPtr stats, - unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p && nr_stats_found < nr_stats) { - if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, + if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, + "actual=", &stats[nr_stats_found]) || + parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ",mem_swapped_in=", &stats[nr_stats_found]) || parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ",mem_swapped_out=", &stats[nr_stats_found]) || @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ",free_mem=", &stats[nr_stats_found]) || parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, - ",total_mem=", &stats[nr_stats_found]) || - parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, - ",actual=", &stats[nr_stats_found])) + ",total_mem=", &stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX "balloon: actual=" +#define BALLOON_PREFIX "balloon: " /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); - if (virStrToLong_ui(offset, &end, 10, &memMB) < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not parse memory balloon allocation from '%s'"), reply); - goto cleanup; + + if (STRPREFIX(offset, "actual=")) { + offset += strlen("actual="); + + if (virStrToLong_ui(offset, &end, 10, &memMB) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("could not parse memory balloon allocation from '%s'"), reply); + goto cleanup; + } + *currmem = memMB * 1024; + ret = 1; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected balloon information '%s'"), reply); + ret = -1; } - *currmem = memMB * 1024; - ret = 1; } else { /* We don't raise an error here, since its to be expected that * many QEMU's don't support ballooning @@ -660,9 +673,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { offset += strlen(BALLOON_PREFIX); - if ((offset = strchr(offset, ',')) != NULL) { - ret = qemuMonitorParseExtraBalloonInfo(offset, stats, nr_stats); - } + ret = qemuMonitorParseBalloonInfo(offset, stats, nr_stats); } VIR_FREE(reply); -- 1.7.6

4 11

[libvirt] [PATCH] build: fix compilation on mingw64
by Eric Blake 19 Aug '11

19 Aug '11

* .gnulib: Update to latest, for getcwd fixes. --- Spurred by a gnulib bug report by Marc-André Lureau. Fedora currently uses the mingw32 cross-compiler, but is aiming to switch to the mingw64 cross-compiler; once that is done, failure to upgrade gnulib would trip up cross-compilation during ./autobuild.sh. * .gnulib 4470580...56d6664 (21): > getcwd: fix test failures on mingw > getcwd-lgpl: fix m4 to match relaxed test for BSD > getcwd: fix compilation on mingw64 > pipe2: silence compiler warning > autoupdate > relocatable-prog: fix link error > getaddrinfo: fix sh typo in gai_strerrorA decl checking > base64: Reference latest RFC. > * build-aux/bootstrap (slurp): Remove obsolescent gettext.m4 patch. > po/Makefile.in.in: fix make -q problem > configmake: fix make -q problem > git-version-gen: correct the advice in a comment > base64: fix off-by-one buffer size bug > closein: correct comments > More tests for 'fseeko'. > ChangeLog: fix wrong attribution in last commit > fseeko: remove unneeded hack > fseeko: fix bug on glibc > unictype/base: Fix interoperability with preinstalled libunistring. > iswblank: Detect declaration correctly. > tcgetsid: Detect declaration correctly. .gnulib | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/.gnulib b/.gnulib index 4470580..56d6664 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 4470580881a7b821b52fb5635102ef3e27aa5af4 +Subproject commit 56d6664559f449af25f0d331457b014b02324d65 -- 1.7.4.4

2 2

[libvirt] [RFC] Add a new feild to qemud_save_header to tell if the saved image is corrupt
by Osier Yang 19 Aug '11

19 Aug '11

If one tries to restore a domain from a corrupt save image, we blindly goes forward to restore from it, this can cause many different errors, depending on how much the image is saved. E.g. https://bugzilla.redhat.com/show_bug.cgi?id=730750 So I'm thinking if we can introduce a new feild to struct qemud_save_header, such as "bool complete;", and set it true if succeeded to save the image, false if not. So that could do some checking while trying to open the image (qemuDomainSaveImageOpen), and quit early if "complete" is false, with a sensiable error message. Thought? Thanks Osier

3 9

[libvirt] [PATCH] storage: Flush host cache after write
by Michal Privoznik 19 Aug '11

19 Aug '11

Although we are flushing cache after some critical writes (e.g. volume creation), after some others we do not (e.g. volume cloning). This patch fix this issue. That is for volume cloning, writing header of logical volume, and storage wipe. --- src/storage/storage_backend.c | 8 ++++++++ src/storage/storage_backend_logical.c | 7 +++++++ src/storage/storage_driver.c | 8 ++++++++ 3 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 6243d1e..889f530 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -208,6 +208,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, } while ((amtleft -= interval) > 0); } + if (fdatasync(fd) < 0) { + ret = -errno; + virReportSystemError(errno, _("cannot sync data to file '%s'"), + vol->target.path); + goto cleanup; + } + + if (VIR_CLOSE(inputfd) < 0) { ret = -errno; virReportSystemError(errno, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index c622d2a..ca4166d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -424,6 +424,13 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_FORCE_CLOSE(fd); goto cleanup; } + if (fsync(fd) < 0) { + virReportSystemError(errno, + _("cannot flush header of device'%s'"), + pool->def->source.devices[i].path); + VIR_FORCE_CLOSE(fd); + goto cleanup; + } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot close device '%s'"), diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6715790..68cac1f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1777,6 +1777,14 @@ storageWipeExtent(virStorageVolDefPtr vol, remaining -= written; } + if (fdatasync(fd) < 0) { + ret = -errno; + virReportSystemError(errno, + _("cannot sync data to volume with path '%s'"), + vol->target.path); + goto out; + } + VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", *bytes_wiped, vol->target.path); -- 1.7.3.4

3 4

[libvirt] [PATCH v2] daemon: initialize GnuTLS
by Michal Privoznik 19 Aug '11

19 Aug '11

When spice_tls is set but listen_tls is not, we don't initialize GnuTLS library. So any later gnutls call (e.g. during migration, where we initialize a certificate) will access uninitialized GnuTLS internal structs and throws an error. Although, we might now initialize GnuTLS twice, it is safe according to the documentation: This function can be called many times, but will only do something the first time. This patch creates 2 functions: virNetTLSInit and virNetTLSDeinit with respect to written above. --- diff to v1: - moved from qemu to daemon - created special init function daemon/libvirtd.c | 2 ++ src/rpc/virnettlscontext.c | 25 ++++++++++++++++++++++--- src/rpc/virnettlscontext.h | 3 +++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b99c637..0530ba5 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1516,6 +1516,7 @@ int main(int argc, char **argv) { virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START, 0, "start", NULL); + virNetTLSInit(); if (daemonSetupNetworking(srv, config, sock_file, sock_file_ro, ipsock, privileged) < 0) { @@ -1554,6 +1555,7 @@ cleanup: virNetServerProgramFree(qemuProgram); virNetServerClose(srv); virNetServerFree(srv); + virNetTLSDeinit(); if (statuswrite != -1) { if (ret != 0) { /* Tell parent of daemon what failed */ diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 19a9b25..8482eaf 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -679,9 +679,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, ctxt->refs = 1; - /* Initialise GnuTLS. */ - gnutls_global_init(); - if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) { int val; if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) @@ -1399,3 +1396,25 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess) virMutexDestroy(&sess->lock); VIR_FREE(sess); } + +/* + * This function MUST be called before any + * virNetTLS* because it initializes + * underlying GnuTLS library. According to + * it's documentation, it's safe to be called + * many times, but is not thread safe. Each + * call SHOULD be later followed by + * virNetTLSContextDeinit. + */ +void virNetTLSInit(void) +{ + gnutls_global_init(); +} + +/* + * See virNetTLSInit + */ +void virNetTLSDeinit(void) +{ + gnutls_global_deinit(); +} diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 641d67e..99f31b9 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -30,6 +30,9 @@ typedef struct _virNetTLSSession virNetTLSSession; typedef virNetTLSSession *virNetTLSSessionPtr; +void virNetTLSInit(void); +void virNetTLSDeinit(void); + virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath, bool tryUserPkiPath, const char *const*x509dnWhitelist, -- 1.7.3.4

2 2

[libvirt] [PATCH] schedinfo: add missing documentation
by Taku Izumi 19 Aug '11

19 Aug '11

This patch adds the missing documentation about the scheduler parameter "vcpu_period" and "vcpu_quota". Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com> --- tools/virsh.pod | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -712,7 +712,9 @@ I<domain-id> Allows you to show (and set) the domain scheduler parameters. The parameters available for each hypervisor are: -LXC, QEMU/KVM (posix scheduler): cpu_shares +LXC (posix scheduler) : cpu_shares + +QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota Xen (credit scheduler): weight, cap @@ -729,6 +731,10 @@ Therefore, -1 is a useful shorthand for B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>. +B<Note>: The vcpu_period parameter has a valid value range of 1000-1000000 or +0, and the vcpu_quota parameter has a valid value range of 1000-1844674407370955 +or less than 0. + =item B<screenshot> I<domain-id> [I<imagefilepath>] [I<--screen> B<screenID>] Takes a screenshot of a current domain console and stores it into a file.

2 1

[libvirt] [PATCH] maint: ignore generated file
by Eric Blake 18 Aug '11

18 Aug '11

I did 'git add .' while in the middle of 'make syntax-check', and it picked up a temporary file that should not be committed. * .gitignore: Ignore sc_* from syntax check. --- Pushing under the trivial rule. .gitignore | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index e8b2dbf..39ecd6d 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,7 @@ /po/* /proxy/ /python/generator.py.stamp +/sc_* /src/libvirt_iohelper /src/locking/qemu-sanlock.conf /src/remote/*_client_bodies.h -- 1.7.4.4

1 0

[libvirt] [RFC] Should libvirt be a proxy for migration data
by Jiri Denemark 18 Aug '11

18 Aug '11

Hi all, Currently when we start a non-tunneled migration, data go straight from source qemu to destination qemu. This is nice in that there is no additional overhead but it also has several disadvantages. If the communication between source and destination qemu breaks, we only get unexpected error message from qemu with no glue about what happened. Another issue is that if qemu cannot send migration data, we cannot cancel the migration because migrate_cancel blocks until all buffers with migration data queued up for transmission are written into the socket. That said, I think we should act as a proxy between source and destination qemu so that we can detect and report normal errors (such as connection reset by peer) and cancel migration at any time. Since we have virNetSocket and we already use that for connecting to destination qemu, we should use it for proxying migration data as well. This approach also has some disadvantages, e.g., a single libvirt thread instead of several qemu processes will now send migration data from all domains that are being migrated. However, I feel like the gain is bigger than the downside. And we already do the same for tunneled migration anyway. Any objections? Jirka

2 2

Re: [libvirt] [Qemu-devel] Killing block migration in qemu?
by Anthony Liguori 17 Aug '11

17 Aug '11

On 08/17/2011 11:51 AM, Paolo Bonzini wrote: > Hi all, > > following discussions yesterday with Juan Quintela and Marcelo Tosatti, > here is my humble proposal: remove block migration from qemu master. +1 I was going to propose the same thing after the conference actually. Regards, Anthony Liguori

1 0

[libvirt] [PATCH 0/5 v2] Some rearrangement & additions to pid file handling/locking
by Daniel P. Berrange 17 Aug '11

17 Aug '11

An update to: https://www.redhat.com/archives/libvir-list/2011-August/msg00349.html Changes in v2: - Fix all docs & code bugs from previous review

4 13