September 2011 - Devel - libvirt List Archives

[libvirt] [PATCH 0/5] avoid memory leaks
by ajia＠redhat.com 20 Sep '11

20 Sep '11

Detected by Coverity. * daemon/remote.c: Calling allocation function 'get_nonnull_domain' to allocate memory to 'dom', but haven't released it in 'cleanup' section. * src/qemu/qemu_command.c: Taking 'if (!port)' true branch then 'def = NULL', the codes jump into 'error' section, the function 'virDomainDefFree(def)' willn't release it. * src/remote/remote_driver.c: Calling allocation function 'get_nonnull_domain' to allocate memory to 'dom', but haven't released it in 'no_memory' section. * src/util/pci.c: Calling allocation function 'pciGetVirtualFunctions' on 'virt_fns', but haven't released it in 'out' section. * src/vmx/vmx.c: Calling allocation function 'virConfReadMem' to allocate memory to 'conf' and then taking 'if (VIR_ALLOC(def) < 0)' true branch, the codes haven't released it before 'return NULL'. Signed-off-by: Alex Jia <ajia(a)redhat.com> --- daemon/remote.c | 2 ++ src/qemu/qemu_command.c | 1 - src/remote/remote_driver.c | 2 ++ src/util/pci.c | 1 + src/vmx/vmx.c | 1 + 5 files changed, 6 insertions(+), 1 deletions(-)

4 12

[libvirt] [PATCH] Remove devname identifier from autogenerated RPC code
by Peter Krempa 20 Sep '11

20 Sep '11

Patch 79cf07a missed one instance of "devname" in source for RPC code generator. --- src/remote/remote_protocol.x | 2 +- src/remote_protocol-structs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4ec1c57..455e324 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2102,7 +2102,7 @@ struct remote_domain_snapshot_delete_args { struct remote_domain_open_console_args { remote_nonnull_domain dom; - remote_string devname; + remote_string dev_name; unsigned int flags; }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 27178da..810b19c 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1585,7 +1585,7 @@ struct remote_domain_snapshot_delete_args { }; struct remote_domain_open_console_args { remote_nonnull_domain dom; - remote_string devname; + remote_string dev_name; u_int flags; }; struct remote_storage_vol_upload_args { -- 1.7.3.4

2 1

[libvirt] [PATCH] virsh: doc: Fix supported driver types for attach-disk command
by Peter Krempa 19 Sep '11

19 Sep '11

Virsh man page lists driver types to be used with attach-device command, but does not specify that those are usable only with the XEN Hypervisor. This patch adds statement, that those options specified are applicable only on the Xen hypervisor and adds option usable with qemu emulator. This patch also changes type of error returned by QEMU driver if the user specifies incompatible driver type from VIR_ERR_INTERNAL_ERROR to VIR_ERR_NO_SUPPORT. --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 2 +- tools/virsh.pod | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e8b1157..9ca8c1b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3657,7 +3657,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, + qemuReportError(VIR_ERR_NO_SUPPORT, _("unsupported driver name '%s' for disk '%s'"), disk->driverName, disk->src); goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e2f428f..602f3e6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5208,7 +5208,7 @@ qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver, int ret = -1; if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, + qemuReportError(VIR_ERR_NO_SUPPORT, _("unsupported driver name '%s' for disk '%s'"), disk->driverName, disk->src); goto end; diff --git a/tools/virsh.pod b/tools/virsh.pod index 02726f3..6b6e7f6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1166,7 +1166,8 @@ the single existing device; consider using B<update-device> for this usage. Attach a new disk device to the domain. I<source> and I<target> are paths for the files and devices. -I<driver> can be I<file>, I<tap> or I<phy> depending on the kind of access. +I<driver> can be I<file>, I<tap> or I<phy> for the Xen hypervisor depending on +the kind of access or I<qemu> for the QEMU emulator. I<type> can indicate I<cdrom> or I<floppy> as alternative to the disk default, although this use only replaces the media within the existing virtual cdrom or floppy device; consider using B<update-device> for this usage instead. -- 1.7.3.4

2 1

[libvirt] [PATCH v7] 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. Example of ordered and "translated" output: PRE-patch: virsh # domblkstat 1 vda vda wr_bytes 5170176 vda wr_operations 511 vda rd_bytes 82815488 vda rd_operations 3726 POST-patch: virsh # domblkstat 1 vda vda rd_req 3726 vda rd_bytes 82815488 vda wr_req 478 vda wr_bytes 4965376 Example of human readable output: virsh # domblkstat 1 vda --human Device: vda number of read operations: 3726 number of read bytes: 82815488 number of write operations: 478 number of bytes written: 4965376 https://bugzilla.redhat.com/show_bug.cgi?id=731656 Changes to v6: - post correct version - add examples of output 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. ( sorry for the mess with v6 :/ ) 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..f2bb2db 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

2 5

[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 19 Sep '11

19 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