- Devel - libvirt List Archives

[libvirt] [PATCH v2 5/5] vmx: avoid memory leak
by ajia＠redhat.com 19 Sep '11

19 Sep '11

* src/vmx/vmx.c: fix memory leak, 'def' has a initial value 'NULL', so 'goto cleanup' is perfected instead of adding a virConfFree before 'return NULL'. Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/vmx/vmx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index dff3599..be91c13 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1253,7 +1253,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) /* Allocate domain def */ if (VIR_ALLOC(def) < 0) { virReportOOMError(); - return NULL; + goto cleanup; } def->virtType = VIR_DOMAIN_VIRT_VMWARE; -- 1.7.1

2 1

[libvirt] [PATCH] Update to require sanlock 1.8 for license compliance
by Daniel P. Berrange 19 Sep '11

19 Sep '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Inexpicably the sanlock code all got placed under the GPLv2-only, so libvirt's use of sanlock introduces a license incompatibility. The sanlock developers have now rearranged the code such that there is a 'sanlock_client.so' which is LGPLv2+ while their daemon remains GPLv2-only. To use the new client library we need to call the new sanlock_init and sanlock_align APIs instead of sanlock_direct_init and sanlock_direct_align. These APIs calls are now routed via the sanlock daemon, instead of doing direct I/O calls to disk. For all this we require sanlock >= 1.8 * configure.ac: Check for sanlock_client.so instead of sanlock.so and fix various comments * libvirt.spec.in: Mandate sanlock >= 1.8 * src/Makefile.am: Link to -lsanlock_client * src/locking/lock_driver_sanlock.c: Use sanlock_init and sanlock_align --- configure.ac | 10 +++++----- libvirt.spec.in | 4 ++-- src/Makefile.am | 2 +- src/locking/lock_driver_sanlock.c | 9 ++++----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index ad9d754..4208cfa 100644 --- a/configure.ac +++ b/configure.ac @@ -966,7 +966,7 @@ AC_SUBST([YAJL_LIBS]) dnl SANLOCK https://fedorahosted.org/sanlock/ AC_ARG_WITH([sanlock], - AC_HELP_STRING([--with-sanlock], [use SANLOCK for lock management @<:@default=check@:>@]), + AC_HELP_STRING([--with-sanlock], [build Sanlock plugin for lock management @<:@default=check@:>@]), [], [with_sanlock=check]) @@ -989,8 +989,8 @@ if test "x$with_sanlock" != "xno"; then fail=1 fi]) if test "x$with_sanlock" != "xno" ; then - AC_CHECK_LIB([sanlock], [sanlock_restrict],[ - SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock" + AC_CHECK_LIB([sanlock_client], [sanlock_init],[ + SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock_client" with_sanlock=yes ],[ if test "x$with_sanlock" = "xcheck" ; then @@ -1001,12 +1001,12 @@ if test "x$with_sanlock" != "xno"; then ]) fi test $fail = 1 && - AC_MSG_ERROR([You must install the SANLOCK development package in order to compile libvirt]) + AC_MSG_ERROR([You must install the Sanlock development package in order to compile libvirt]) CPPFLAGS="$old_cppflags" LIBS="$old_libs" if test "x$with_sanlock" = "xyes" ; then AC_DEFINE_UNQUOTED([HAVE_SANLOCK], 1, - [whether SANLOCK is available for JSON parsing/formatting]) + [whether Sanlock plugin for lock management is available]) fi fi AM_CONDITIONAL([HAVE_SANLOCK], [test "x$with_sanlock" = "xyes"]) diff --git a/libvirt.spec.in b/libvirt.spec.in index 25e521c..f3f6722 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -354,7 +354,7 @@ BuildRequires: libpciaccess-devel >= 0.10.9 BuildRequires: yajl-devel %endif %if %{with_sanlock} -BuildRequires: sanlock-devel +BuildRequires: sanlock-devel >= 1.8 %endif %if %{with_libpcap} BuildRequires: libpcap-devel @@ -518,7 +518,7 @@ the virtualization capabilities of recent versions of Linux (and other OSes). %package lock-sanlock Summary: Sanlock lock manager plugin for QEMU driver Group: Development/Libraries -Requires: sanlock +Requires: sanlock >= 1.8 #for virt-sanlock-cleanup require augeas Requires: augeas Requires: %{name} = %{version}-%{release} diff --git a/src/Makefile.am b/src/Makefile.am index 9a903ea..738ee91 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1279,7 +1279,7 @@ lockdriver_LTLIBRARIES = sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = $(AM_CLFAGS) sanlock_la_LDFLAGS = -module -avoid-version -sanlock_la_LIBADD = -lsanlock \ +sanlock_la_LIBADD = -lsanlock_client \ ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_sanlock.aug diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index b93fe01..2d72510 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -33,7 +33,6 @@ #include <sanlock.h> #include <sanlock_resource.h> -#include <sanlock_direct.h> #include <sanlock_admin.h> #include "lock_driver.h" @@ -181,7 +180,7 @@ static int virLockManagerSanlockSetupLockspace(void) } VIR_DEBUG("Someone else just created lockspace %s", path); } else { - if ((rv = sanlock_direct_align(&ls.host_id_disk)) < 0) { + if ((rv = sanlock_align(&ls.host_id_disk)) < 0) { if (rv <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Unable to query sector size %s: error %d"), @@ -210,7 +209,7 @@ static int virLockManagerSanlockSetupLockspace(void) goto error_unlink; } - if ((rv = sanlock_direct_init(&ls, NULL, 0, 0, 0)) < 0) { + if ((rv = sanlock_init(&ls, NULL, 0, 0)) < 0) { if (rv <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Unable to initialize lockspace %s: error %d"), @@ -555,7 +554,7 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res) } VIR_DEBUG("Someone else just created lockspace %s", res->disks[0].path); } else { - if ((rv = sanlock_direct_align(&res->disks[0])) < 0) { + if ((rv = sanlock_align(&res->disks[0])) < 0) { if (rv <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Unable to query sector size %s: error %d"), @@ -584,7 +583,7 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res) goto error_unlink; } - if ((rv = sanlock_direct_init(NULL, res, 0, 0, 0)) < 0) { + if ((rv = sanlock_init(NULL, res, 0, 0)) < 0) { if (rv <= -200) virLockError(VIR_ERR_INTERNAL_ERROR, _("Unable to initialize lease %s: error %d"), -- 1.7.6

2 1

[libvirt] [PATCH] spec: Require augeas for sanlock
by Michal Privoznik 19 Sep '11

19 Sep '11

Even though we BuildRequire augeas in some cases, we need to require it even after if we build with sanlock. virt-sanlock-cleanup use it. --- libvirt.spec.in | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 648b50f..4d69c77 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -354,6 +354,8 @@ BuildRequires: libpciaccess-devel >= 0.10.9 BuildRequires: yajl-devel %endif %if %{with_sanlock} +#for virt-sanlock-cleanup +Requires: augeas BuildRequires: sanlock-devel %endif %if %{with_libpcap} -- 1.7.3.4

3 7

[libvirt] [PATCH v6] virsh: Add more human-friendly output of domblkstat command
by Peter Krempa 19 Sep '11

19 Sep '11

Users of virsh complain that output of the domblkstat command is not intuitive enough. This patch adds explanation of fields returned by this command to the help section for domblkstat and the man page of virsh. Also a switch --human is added for domblkstat that prints the fields with more descriptive texts. This patch also changes sequence of the output fields and their names back to the order and spelling established by previous versions of virsh to mantain compatibility with scripts. https://bugzilla.redhat.com/show_bug.cgi?id=731656 Changes to v5: - Do not fix VIR_ERR_RPC failure of this function (unknown procedure 243) ( will be fixed in a separate patch adressing more similar errors) - Clean up legacy printout using a macro - Remove newlines from "desc" in vshCmdInfo - use calloc() instead of malloc() + memset(..., 0, ...) Changes to v4: - Fields are printed in order established by previous versions - Fix translation macros - Run old api unconditionaly (while comunicating with old libvirtd "error: unknown procedure: 243" occured) - Remove redundant field description from command help and add reference to man page. - Fix formating (unnecessary space after function name) - Fix man page, mentioning that fields may be missig. Changes to v3: - Add units to duration values - Add missing write doc/stranslation - Add translation from new api names to legacy names used in previous versions in virsh Changes to v2: - Modify for new fields in virDomainBlockStatsFlags Changes to v1: - Rebase to current head --- IMHO this patch is worth getting into 0.9.5 as it fixes the changed output order since addition of the new api, and translates field names into those that have been used in previous versions. tools/virsh.c | 236 +++++++++++++++++++++++++++++++++++++++++------------- tools/virsh.pod | 20 +++++- 2 files changed, 198 insertions(+), 58 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d575425..6f190b0 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -369,6 +369,8 @@ static const char *vshDomainStateReasonToString(int state, int reason); static const char *vshDomainControlStateToString(int state); static const char *vshDomainVcpuStateToString(int state); static bool vshConnectionUsability(vshControl *ctl, virConnectPtr conn); +static virTypedParameterPtr vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count); +static char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item); static char *editWriteToTempFile (vshControl *ctl, const char *doc); static int editFile (vshControl *ctl, const char *filename); @@ -1054,16 +1056,43 @@ cleanup: */ static const vshCmdInfo info_domblkstat[] = { {"help", N_("get device block stats for a domain")}, - {"desc", N_("Get device block stats for a running domain.")}, + {"desc", N_("Get device block stats for a running domain. See man page or use --human for explanation of fields")}, {NULL,NULL} }; static const vshCmdOptDef opts_domblkstat[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")}, + {"human", VSH_OT_BOOL, 0, N_("print a more human readable output")}, {NULL, 0, 0, NULL} }; +struct _domblkstat_sequence { + const char *field; /* field name */ + const char *legacy; /* legacy name from previous releases */ + const char *human; /* human-friendly explanation */ +}; + +/* sequence of values for output to honor legacy format from previous versions */ +static const struct _domblkstat_sequence domblkstat_output[] = { + { VIR_DOMAIN_BLOCK_STATS_READ_REQ, "rd_req", N_("number of read operations: ") }, /* 0 */ + { VIR_DOMAIN_BLOCK_STATS_READ_BYTES, "rd_bytes", N_("number of read bytes: ") }, /* 1 */ + { VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, "wr_req", N_("number of write operations: ") }, /* 2 */ + { VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, "wr_bytes", N_("number of bytes written: ") }, /* 3 */ + { VIR_DOMAIN_BLOCK_STATS_ERRS, "errs", N_("error count: ") }, /* 4 */ + { VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ, NULL, N_("number of flush operations: ") }, /* 5 */ + { VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES, NULL, N_("total duration of reads (ns): ") }, /* 6 */ + { VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, NULL, N_("total duration of writes (ns): ") }, /* 7 */ + { VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES, NULL, N_("total duration of flushes (ns):") }, /* 8 */ + { NULL, NULL, NULL } +}; + +#define DOMBLKSTAT_LEGACY_PRINT(ID, VALUE) \ + if (VALUE >= 0) \ + vshPrint(ctl, "%s %s %lld\n", device, \ + human?_(domblkstat_output[ID].human):domblkstat_output[ID].legacy\ + VALUE); + static bool cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) { @@ -1071,8 +1100,13 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) const char *name = NULL, *device = NULL; struct _virDomainBlockStats stats; virTypedParameterPtr params = NULL; + virTypedParameterPtr par = NULL; + char *value = NULL; + const char *field = NULL; int rc, nparams = 0; + int i = 0; bool ret = false; + bool human = vshCommandOptBool(cmd, "human"); /* enable human readable output */ if (!vshConnectionUsability (ctl, ctl->conn)) return false; @@ -1090,76 +1124,84 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) * then. */ if (rc < 0) { - if (last_error->code != VIR_ERR_NO_SUPPORT) { - virshReportError(ctl); + /* try older API if newer is not supported */ + if (last_error->code != VIR_ERR_NO_SUPPORT) goto cleanup; - } else { - virFreeError(last_error); - last_error = NULL; - - if (virDomainBlockStats (dom, device, &stats, - sizeof stats) == -1) { - vshError(ctl, _("Failed to get block stats %s %s"), - name, device); - goto cleanup; - } - - if (stats.rd_req >= 0) - vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req); - if (stats.rd_bytes >= 0) - vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes); - - if (stats.wr_req >= 0) - vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req); + virFreeError(last_error); + last_error = NULL; - if (stats.wr_bytes >= 0) - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes); + if (virDomainBlockStats (dom, device, &stats, + sizeof stats) == -1) { + vshError(ctl, _("Failed to get block stats %s %s"), + name, device); + goto cleanup; + } - if (stats.errs >= 0) - vshPrint (ctl, "%s errs %lld\n", device, stats.errs); + /* human friendly output */ + if (human) { + vshPrint(ctl, N_("Device: %s\n"), device); + device = ""; } + + DOMBLKSTAT_LEGACY_PRINT(0, stats.rd_req); + DOMBLKSTAT_LEGACY_PRINT(1, stats.rd_bytes); + DOMBLKSTAT_LEGACY_PRINT(2, stats.wr_req); + DOMBLKSTAT_LEGACY_PRINT(3, stats.wr_bytes); + DOMBLKSTAT_LEGACY_PRINT(4, stats.errs); } else { - params = vshMalloc(ctl, sizeof(*params) * nparams); - memset(params, 0, sizeof(*params) * nparams); + params = vshCalloc(ctl, nparams, sizeof(*params)); if (virDomainBlockStatsFlags (dom, device, params, &nparams, 0) < 0) { vshError(ctl, _("Failed to get block stats %s %s"), name, device); goto cleanup; } - int i; - /* XXX: The output sequence will be different. */ + /* set for prettier output */ + if (human) { + vshPrint(ctl, N_("Device: %s\n"), device); + device = ""; + } + + /* at first print all known values in desired order */ + for (i = 0; domblkstat_output[i].field != NULL; i++) { + if (!(par = vshFindTypedParamByName(domblkstat_output[i].field, + params, + nparams))) + continue; + + if (!(value = vshGetTypedParamValue(ctl, par))) + continue; + + /* to print other not supported fields, mark the already printed */ + par->field[0] = '\0'; /* set the name to empty string */ + + /* translate into human readable or legacy spelling */ + field = NULL; + if (human) + field = _(domblkstat_output[i].human); + else + field = domblkstat_output[i].legacy; + + /* use the provided spelling if no translation is available */ + if (!field) + field = domblkstat_output[i].field; + + vshPrint(ctl, "%s %s %s\n", device, field, value); + + VIR_FREE(value); + } + + /* go through the fields again, looking for fields that were not printed*/ for (i = 0; i < nparams; i++) { - switch(params[i].type) { - case VIR_TYPED_PARAM_INT: - vshPrint (ctl, "%s %s %d\n", device, - params[i].field, params[i].value.i); - break; - case VIR_TYPED_PARAM_UINT: - vshPrint (ctl, "%s %s %u\n", device, - params[i].field, params[i].value.ui); - break; - case VIR_TYPED_PARAM_LLONG: - vshPrint (ctl, "%s %s %lld\n", device, - params[i].field, params[i].value.l); - break; - case VIR_TYPED_PARAM_ULLONG: - vshPrint (ctl, "%s %s %llu\n", device, - params[i].field, params[i].value.ul); - break; - case VIR_TYPED_PARAM_DOUBLE: - vshPrint (ctl, "%s %s %f\n", device, - params[i].field, params[i].value.d); - break; - case VIR_TYPED_PARAM_BOOLEAN: - vshPrint (ctl, "%s %s %s\n", device, - params[i].field, params[i].value.b ? _("yes") : _("no")); - break; - default: - vshError(ctl, _("unimplemented block statistics parameter type")); - } + if (strlen(params[i].field) == 0) + continue; + if (!(value = vshGetTypedParamValue(ctl, params+i))) + continue; + + vshPrint(ctl, "%s %s %s\n", device, params[i].field, value); + VIR_FREE(value); } } @@ -1170,6 +1212,7 @@ cleanup: virDomainFree(dom); return ret; } +#undef DOMBLKSTAT_LEGACY_PRINT /* "domifstat" command */ @@ -15202,6 +15245,85 @@ vshDomainStateReasonToString(int state, int reason) return N_("unknown"); } +static char * +vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) +{ + int size = 0; + char *str = NULL; + + if (!ctl || !item) + return NULL; + + switch(item->type) { + case VIR_TYPED_PARAM_INT: + size = snprintf(NULL, 0, "%d", item->value.i); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%d", item->value.i); + return str; + break; + + case VIR_TYPED_PARAM_UINT: + size = snprintf(NULL, 0, "%u", item->value.ui); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%u", item->value.ui); + return str; + break; + + case VIR_TYPED_PARAM_LLONG: + size = snprintf(NULL, 0, "%lld", item->value.l); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%lld", item->value.l); + return str; + break; + + case VIR_TYPED_PARAM_ULLONG: + size = snprintf(NULL, 0, "%llu", item->value.ul); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%llu", item->value.ul); + return str; + break; + + case VIR_TYPED_PARAM_DOUBLE: + size = snprintf(NULL, 0, "%f", item->value.d); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%f", item->value.d); + return str; + break; + + case VIR_TYPED_PARAM_BOOLEAN: + size = snprintf(NULL, 0, "%s", item->value.b ? _("yes") : _("no")); + str = vshMalloc(ctl, size+1); + snprintf(str, size+1, "%s", item->value.b ? _("yes") : _("no")); + return str; + break; + + default: + vshError(ctl, _("unimplemented block statistics parameter type")); + } + + return NULL; +} + +static virTypedParameterPtr +vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count) +{ + int i = count; + virTypedParameterPtr found = list; + + if (!list || !name) + return NULL; + + while (i-- > 0) { + if (STREQ(name, found->field)) + return found; + + found++; /* go to next struct in array */ + } + + /* not found */ + return NULL; +} + static const char * vshDomainControlStateToString(int state) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 02726f3..edf451f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -501,13 +501,31 @@ be lost once the guest stops running, but the snapshot contents still exist, and a new domain with the same name and UUID can restore the snapshot metadata with B<snapshot-create>. -=item B<domblkstat> I<domain> I<block-device> +=item B<domblkstat> I<domain> I<block-device> [I<--human>] Get device block stats for a running domain. A I<block-device> corresponds to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to I<domain> (see also B<domblklist> for listing these names). +Use I<--human> for a more human readable output. + +Availabilty of these fields depends on hypervisor. Unsupported fields are +missing from the output. Other fields may appear if comunicating with a newer +version of libvirtd. + +B<Explanation of fields> (fields appear in the folowing order): + rd_req - count of read operations + rd_bytes - count of read bytes + wr_req - count of write operations + wr_bytes - count of written bytes + errs - error count + flush_operations - count of flush operations + rd_total_times - total time read operations took (ns) + wr_total_times - total time write operations took (ns) + flush_total_times - total time flush operations took (ns) + <-- other fields provided by hypervisor --> + =item B<domifstat> I<domain> I<interface-device> Get network interface stats for a running domain. -- 1.7.3.4

1 1

[libvirt] [RFC] API to invoke S3/S4 on a KVM host and also resume from within libvirt
by Srivatsa S. Bhat 18 Sep '11

18 Sep '11

Hi, In order to exploit the host power management capabilities like S3/S4, it was proposed to add additional tags in the libvirt capabilities XML to export the power management features supported by the host. The v5 of the RFC patch is here: http://www.redhat.com/archives/libvir-list/2011-August/msg00324.html. It was also decided that it would be better to invoke S3/S4 and resume the host, from within libvirt, instead of doing it out-of-band. And it was agreed upon that an RTC wake up mechanism to resume the host from sleep would be best suited for doing the resume in-band. The discussion can be found here: http://www.redhat.com/archives/libvir-list/2011-August/msg00327.html An API for this was initially discussed here: http://www.redhat.com/archives/libvir-list/2011-August/msg00248.html One of the key challenges in implementing the API is that it has to return success to the caller before the host gets suspended, otherwise the connection will get terminated and hence the status cannot be sent across to the caller. So to achieve this, we would need some kind of asynchronous mechanism. Here are some ideas on how to go about doing this: 1. Using the virCommandRunAsync() API to run the rtcwake command. For example, int virNodeSuspendForDuration(virConnectPtr conn, unsigned flags, long duration) { virCommandPtr cmd; //Build up the rtcwake command with appropriate arguments, in 'cmd' virCommandRunAsync(cmd, NULL); return 0; //Success } Issues with this approach: We cannot be sure that the node will not get suspended before returning the status to the caller. 2. Using a timeout to delay the invocation of suspend. We can use a pre-exec-hook to cause that delay. For example, int delay = 5; //in seconds int virNodeSuspendForDuration(virConnectPtr conn, unsigned flags, long duration) { virCommandPtr cmd; //Build up the rtcwake command with appropriate arguments, in 'cmd' virCommandSetPreExecHook(cmd, virNodeSuspendHook, &delay); virCommandRunAsync(cmd, NULL); return 0; //Success } int virNodeSuspendHook(void *data) { int *delay = data; /* Just delay for some time */ sleep(*delay); return 0; } Issues with this approach: We may have to choose the delay value based on heuristics. But I don't see any serious downsides to this as long as the delay is a suitably large value. Please let me know your suggestions. -- Regards, Srivatsa S. Bhat <srivatsa.bhat(a)linux.vnet.ibm.com> Linux Technology Center, IBM India Systems and Technology Lab

2 1

[libvirt] [PATCH] virsh: Use old API if remote libvirtd does not support new
by Peter Krempa 18 Sep '11

18 Sep '11

Commit ffe28ab74b821c916ec4ba8efb5c992454e4bd24 introduced regression while comunicating with older libvirtd command 'domblkstat' used the new API and did not check for VIR_ERR_RPC error code signalling the remote server does not support this API and did not fall back to older API. Thereafter 'domblkstat' ended with "error: unknown procedure: 243". --- footnote: Will not apply along with: [libvirt] [PATCH v5] virsh: Add more human-friendly output of domblkstat command as that path does fix this bug. tools/virsh.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3b060bf..430168c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1090,7 +1090,8 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) * then. */ if (rc < 0) { - if (last_error->code != VIR_ERR_NO_SUPPORT) { + if (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_RPC) { virshReportError(ctl); goto cleanup; } else { -- 1.7.3.4

3 8

[libvirt] [PATCH] Fix buzzilla 738778
by Stefan Berger 17 Sep '11

17 Sep '11

This patch fixes the bug shown in bugzilla 738778. It's not an nwfilter problem but a connection sharing / closure issue. https://bugzilla.redhat.com/show_bug.cgi?id=738778 Depending on the speed / #CPUs of the machine you are using you may not see this bug all the time. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8a8475..1991777 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2539,6 +2539,10 @@ struct qemuProcessReconnectData { /* * Open an existing VM's monitor, re-detect VCPU threads * and re-reserve the security labels in use + * + * We own the virConnectPtr we are passed here - whoever started + * this thread function has increased the reference counter to it + * so that we now have to close it. */ static void qemuProcessReconnect(void *opaque) @@ -2632,6 +2636,8 @@ qemuProcessReconnect(void *opaque) qemuDriverUnlock(driver); + virConnectClose(conn); + return; error: @@ -2656,6 +2662,8 @@ error: virDomainObjUnlock(obj); } qemuDriverUnlock(driver); + + virConnectClose(conn); } static void @@ -2706,7 +2714,16 @@ qemuProcessReconnectHelper(void *payload, if (qemuDomainObjBeginJobWithDriver(src->driver, obj, QEMU_JOB_MODIFY) < 0) goto error; + /* Since we close the connection later on, we have to make sure + * that the threads we start see a valid connection throughout their + * lifetime. We simply increase the reference counter here. + */ + virConnectRef(data->conn); + if (virThreadCreate(&thread, true, qemuProcessReconnect, data) < 0) { + + virConnectClose(data->conn); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create thread. QEMU initialization " "might be incomplete"));

3 3

[libvirt] [PATCH] qemu: add return value check
by ajia＠redhat.com 17 Sep '11

17 Sep '11

From: Alex Jia <ajia(a)redhat.com> * src/qemu/qemu_command.c: missing return value check. Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/qemu/qemu_command.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5f0de40..e8b1157 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1702,7 +1702,8 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk, virBufferAsprintf(&opt, ",event_idx=%s", virDomainVirtioEventIdxTypeToString(disk->event_idx)); } - qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps); + if (qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_DISK_BUS_USB: virBufferAddLit(&opt, "usb-storage"); @@ -1781,7 +1782,9 @@ qemuBuildFSDevStr(virDomainFSDefPtr fs, virBufferAsprintf(&opt, ",id=%s", fs->info.alias); virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); - qemuBuildDeviceAddressStr(&opt, &fs->info, qemuCaps); + + if (qemuBuildDeviceAddressStr(&opt, &fs->info, qemuCaps) < 0) + goto error; if (virBufferError(&opt)) { virReportOOMError(); -- 1.7.1

3 2

[libvirt] [PATCH] snapshot: affect persistent xml after disk snapshot
by Eric Blake 17 Sep '11

17 Sep '11

For external snapshots to be useful on persistent domains, we must alter the persistent definition alongside the running definition. Thanks to the possibility of disk hotplug as well as of edits that only affect the persistent xml, we can't assume that vm->def and vm->newDef have the same disk at the same index, so we can only update the persistent copy if the device destination matches up. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive) (qemuDomainSnapshotCreateSingleDiskActive): Also affect newDef, if present. --- This is worth including in 0.9.5 - without it, the new feature of disk snapshots on a persistent domain are lost the moment the domain stops running, which is likely to cause data corruption for guests. I'm still trying to reproduce another reported snapshot failure: https://bugzilla.redhat.com/show_bug.cgi?id=738676 although I don't think this patch will help that issue. src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++------ 1 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 32f376ec..cbe28d8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9107,12 +9107,15 @@ static int qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, virDomainObjPtr vm, virDomainSnapshotDiskDefPtr snap, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + virDomainDiskDefPtr persistDisk) { qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; char *source = NULL; char *driverType = NULL; + char *persistSource = NULL; + char *persistDriverType = NULL; int ret = -1; int fd = -1; char *origsrc = NULL; @@ -9128,7 +9131,11 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || !(source = strdup(snap->file)) || (STRNEQ_NULLABLE(disk->driverType, "qcow2") && - !(driverType = strdup("qcow2")))) { + !(driverType = strdup("qcow2"))) || + (persistDisk && + (!(persistSource = strdup(source)) || + (STRNEQ_NULLABLE(persistDisk->driverType, "qcow2") && + !(persistDriverType = strdup("qcow2")))))) { virReportOOMError(); goto cleanup; } @@ -9176,9 +9183,16 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, disk->driverType = driverType; driverType = NULL; } - - /* XXX Do we also need to update vm->newDef if there are pending - * configuration changes awaiting the next boot? */ + if (persistDisk) { + VIR_FREE(persistDisk->src); + persistDisk->src = persistSource; + persistSource = NULL; + if (persistDriverType) { + VIR_FREE(persistDisk->driverType); + persistDisk->driverType = persistDriverType; + persistDriverType = NULL; + } + } cleanup: if (origsrc) { @@ -9190,6 +9204,8 @@ cleanup: VIR_FREE(device); VIR_FREE(source); VIR_FREE(driverType); + VIR_FREE(persistSource); + VIR_FREE(persistDriverType); return ret; } @@ -9235,12 +9251,22 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, * SNAPSHOT_EXTERNAL with a valid file name and qcow2 format. */ qemuDomainObjEnterMonitorWithDriver(driver, vm); for (i = 0; i < snap->def->ndisks; i++) { + virDomainDiskDefPtr persistDisk = NULL; + if (snap->def->disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO) continue; + if (vm->newDef) { + int indx = virDomainDiskIndexByName(vm->newDef, + vm->def->disks[i]->dst, + false); + if (indx >= 0) + persistDisk = vm->newDef->disks[indx]; + } ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, &snap->def->disks[i], - vm->def->disks[i]); + vm->def->disks[i], + persistDisk); if (ret < 0) break; } -- 1.7.4.4

2 3

[libvirt] [PATCH] snapshot: allow disk snapshots of qcow2 disks
by Eric Blake 17 Sep '11

17 Sep '11

For all types of disks other than qcow2, we were requesting that SELinux labeling visit the new file as if it were qcow2, which means labeling would try to find the backing files of an empty file. And for a pre-existing qcow2 disk, we were passing NULL, which meant that labelling tried to probe the file type (and if probing is disabled, per the default qemu.conf, this made snapshots fail). What we really want is to make SELinux labeling visit the new file as raw; it will later be converted to qcow2 if qemu successfully made the snapshot. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateSingleDiskActive): Force SELinux labeling to avoid probe of new file. --- Fixes https://bugzilla.redhat.com/show_bug.cgi?id=738676 I wonder why we are passing disk types as strings, instead of encoding them as an enum. Use of an enum would avoid my ugly hack of having to cast away const of my temporary assignment of a string. src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2a1e5ea..e2f428f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9151,7 +9151,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, origsrc = disk->src; disk->src = source; origdriver = disk->driverType; - disk->driverType = driverType; + disk->driverType = (char *) "raw"; /* Don't want to probe backing files */ if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0) goto cleanup; -- 1.7.4.4

2 2