- Devel - libvirt List Archives

[libvirt] [PATCH] conf: avoid memory leak on virDomainDefParseXML
by ajia＠redhat.com 16 Sep '11

16 Sep '11

From: Alex Jia <ajia(a)redhat.com> * conf/domain_conf.c: allocate memory to def->redirdevs in virDomainDefParseXML such as VIR_ALLOC_N(def->redirdevs, n), however, virDomainDefFree(def) hasn't released these memory. * Detected in valgrind run: ==19820== 209 (16 direct, 193 indirect) bytes in 1 blocks are definitely lost in loss record 25 of 26 ==19820== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==19820== by 0x4A13AF: virAllocN (memory.c:129) ==19820== by 0x4D4A0E: virDomainDefParseXML (domain_conf.c:7258) ==19820== by 0x4D4C93: virDomainDefParseNode (domain_conf.c:7512) ==19820== by 0x4D562F: virDomainDefParse (domain_conf.c:7465) ==19820== by 0x415863: testCompareXMLToXMLFiles (qemuxml2xmltest.c:35) ==19820== by 0x415982: testCompareXMLToXMLHelper (qemuxml2xmltest.c:80) ==19820== by 0x416D31: virtTestRun (testutils.c:140) ==19820== by 0x415604: mymain (qemuxml2xmltest.c:192) ==19820== by 0x416437: virtTestMain (testutils.c:689) ==19820== by 0x3CA7A1ECDC: (below main) (in /lib64/libc-2.12.so) ==19820== ==19820== LEAK SUMMARY: ==19820== definitely lost: 16 bytes in 1 blocks ==19820== indirectly lost: 193 bytes in 5 blocks ==19820== possibly lost: 0 bytes in 0 blocks ==19820== still reachable: 1,054 bytes in 21 blocks * How to reproduce? % valgrind -v --leak-check=full ./tests/qemuxml2xmltest Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/conf/domain_conf.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ea6b581..03ab447 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1206,6 +1206,10 @@ void virDomainDefFree(virDomainDefPtr def) virDomainHubDefFree(def->hubs[i]); VIR_FREE(def->hubs); + for (i = 0 ; i < def->nredirdevs ; i++) + virDomainRedirdevDefFree(def->redirdevs[i]); + VIR_FREE(def->redirdevs); + VIR_FREE(def->os.type); VIR_FREE(def->os.arch); VIR_FREE(def->os.machine); -- 1.7.1

2 1

[libvirt] [PATCH] Avoid using "devname" as an identifier.
by Peter Krempa 16 Sep '11

16 Sep '11

/usr/lib/stdlib.h in Mac OS X and probably also in BSD's exports this symbol :( --- src/conf/domain_audit.c | 8 +++--- src/driver.h | 2 +- src/libvirt.c | 12 +++++----- src/lxc/lxc_controller.c | 4 +- src/lxc/lxc_driver.c | 6 ++-- src/network/bridge_driver.c | 14 ++++++------ src/qemu/qemu_command.c | 28 +++++++++++++------------- src/qemu/qemu_driver.c | 14 ++++++------ src/qemu/qemu_monitor.c | 36 +++++++++++++++++----------------- src/qemu/qemu_monitor.h | 8 +++--- src/qemu/qemu_monitor_json.c | 26 ++++++++++++------------ src/qemu/qemu_monitor_json.h | 8 +++--- src/qemu/qemu_monitor_text.c | 30 ++++++++++++++-------------- src/qemu/qemu_monitor_text.h | 8 +++--- src/storage/storage_backend_disk.c | 14 ++++++------ src/storage/storage_backend_mpath.c | 8 +++--- src/uml/uml_driver.c | 8 +++--- src/util/util.c | 6 ++-- src/util/util.h | 2 +- src/xen/xen_driver.c | 6 ++-- tools/console.c | 4 +- tools/console.h | 2 +- 22 files changed, 127 insertions(+), 127 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 581be70..16937dc 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -202,7 +202,7 @@ virDomainAuditNetDevice(virDomainDefPtr vmDef, virDomainNetDefPtr netDef, char uuidstr[VIR_UUID_STRING_BUFLEN]; char macstr[VIR_MAC_STRING_BUFLEN]; char *vmname; - char *devname; + char *dev_name; char *rdev; const char *virt; @@ -211,7 +211,7 @@ virDomainAuditNetDevice(virDomainDefPtr vmDef, virDomainNetDefPtr netDef, rdev = virDomainAuditGetRdev(device); if (!(vmname = virAuditEncode("vm", vmDef->name)) || - !(devname = virAuditEncode("path", device))) { + !(dev_name = virAuditEncode("path", device))) { VIR_WARN("OOM while encoding audit message"); goto cleanup; } @@ -223,11 +223,11 @@ virDomainAuditNetDevice(virDomainDefPtr vmDef, virDomainNetDefPtr netDef, VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success, "virt=%s resrc=net reason=open %s uuid=%s net='%s' %s rdev=%s", - virt, vmname, uuidstr, macstr, devname, VIR_AUDIT_STR(rdev)); + virt, vmname, uuidstr, macstr, dev_name, VIR_AUDIT_STR(rdev)); cleanup: VIR_FREE(vmname); - VIR_FREE(devname); + VIR_FREE(dev_name); VIR_FREE(rdev); } diff --git a/src/driver.h b/src/driver.h index fc7a931..3792003 100644 --- a/src/driver.h +++ b/src/driver.h @@ -612,7 +612,7 @@ typedef virDomainPtr typedef int (*virDrvDomainOpenConsole)(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags); diff --git a/src/libvirt.c b/src/libvirt.c index c32c7a6..66c2327 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16312,13 +16312,13 @@ virDomainSnapshotFree(virDomainSnapshotPtr snapshot) /** * virDomainOpenConsole: * @dom: a domain object - * @devname: the console, serial or parallel port device alias, or NULL + * @dev_name: the console, serial or parallel port device alias, or NULL * @st: a stream to associate with the console * @flags: unused, pass 0 * * This opens the backend associated with a console, serial or * parallel port device on a guest, if the backend is supported. - * If the @devname is omitted, then the first console or serial + * If the @dev_name is omitted, then the first console or serial * device is opened. The console is associated with the passed * in @st stream, which should have been opened in non-blocking * mode for bi-directional I/O. @@ -16326,14 +16326,14 @@ virDomainSnapshotFree(virDomainSnapshotPtr snapshot) * returns 0 if the console was opened, -1 on error */ int virDomainOpenConsole(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags) { virConnectPtr conn; - VIR_DOMAIN_DEBUG(dom, "devname=%s, st=%p, flags=%x", - NULLSTR(devname), st, flags); + VIR_DOMAIN_DEBUG(dom, "dev_name=%s, st=%p, flags=%x", + NULLSTR(dev_name), st, flags); virResetLastError(); @@ -16351,7 +16351,7 @@ int virDomainOpenConsole(virDomainPtr dom, if (conn->driver->domainOpenConsole) { int ret; - ret = conn->driver->domainOpenConsole(dom, devname, st, flags); + ret = conn->driver->domainOpenConsole(dom, dev_name, st, flags); if (ret < 0) goto error; return ret; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 5028251..52d382e 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -67,7 +67,7 @@ struct cgroup_device_policy { }; -static int lxcGetLoopFD(char **devname) +static int lxcGetLoopFD(char **dev_name) { int fd = -1; DIR *dh = NULL; @@ -122,7 +122,7 @@ static int lxcGetLoopFD(char **devname) cleanup: if (fd != -1) { VIR_DEBUG("Got free loop device %s %d", looppath, fd); - *devname = looppath; + *dev_name = looppath; } else { VIR_DEBUG("No free loop devices available"); VIR_FREE(looppath); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 6b716c1..6cf7203 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2837,7 +2837,7 @@ cleanup: static int lxcDomainOpenConsole(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags) { @@ -2864,7 +2864,7 @@ lxcDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (devname) { + if (dev_name) { /* XXX support device aliases in future */ lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Named device aliases are not supported")); @@ -2884,7 +2884,7 @@ lxcDomainOpenConsole(virDomainPtr dom, if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { lxcError(VIR_ERR_INTERNAL_ERROR, - _("character device %s is not using a PTY"), devname); + _("character device %s is not using a PTY"), dev_name); goto cleanup; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index cea8771..9960745 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3142,7 +3142,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) virNetworkIpDefPtr ipdef; virSocketAddr addr; virSocketAddrPtr addrptr = NULL; - char *devname = NULL; + char *dev_name = NULL; *netaddr = NULL; networkDriverLock(driver); @@ -3172,7 +3172,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) break; case VIR_NETWORK_FORWARD_BRIDGE: - if ((devname = netdef->bridge)) + if ((dev_name = netdef->bridge)) break; /* * fall through if netdef->bridge wasn't set, since this is @@ -3182,9 +3182,9 @@ networkGetNetworkAddress(const char *netname, char **netaddr) case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: if ((netdef->nForwardIfs > 0) && netdef->forwardIfs) - devname = netdef->forwardIfs[0].dev; + dev_name = netdef->forwardIfs[0].dev; - if (!devname) { + if (!dev_name) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' has no associated interface or bridge"), netdef->name); @@ -3192,11 +3192,11 @@ networkGetNetworkAddress(const char *netname, char **netaddr) break; } - if (devname) { - if (ifaceGetIPAddress(devname, &addr)) { + if (dev_name) { + if (ifaceGetIPAddress(dev_name, &addr)) { virReportSystemError(errno, _("Failed to get IP address for '%s' (network '%s')"), - devname, netdef->name); + dev_name, netdef->name); } else { addrptr = &addr; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 153647a..5f0de40 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -422,20 +422,20 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def) /* Names used before -drive existed */ static int qemuAssignDeviceDiskAliasLegacy(virDomainDiskDefPtr disk) { - char *devname; + char *dev_name; if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && STREQ(disk->dst, "hdc")) - devname = strdup("cdrom"); + dev_name = strdup("cdrom"); else - devname = strdup(disk->dst); + dev_name = strdup(disk->dst); - if (!devname) { + if (!dev_name) { virReportOOMError(); return -1; } - disk->info.alias = devname; + disk->info.alias = dev_name; return 0; } @@ -465,7 +465,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) { int busid, devid; int ret; - char *devname; + char *dev_name; if (virDiskNameToBusDeviceIndex(disk, &busid, &devid) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -477,24 +477,24 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: if (disk->device== VIR_DOMAIN_DISK_DEVICE_DISK) - ret = virAsprintf(&devname, "ide%d-hd%d", busid, devid); + ret = virAsprintf(&dev_name, "ide%d-hd%d", busid, devid); else - ret = virAsprintf(&devname, "ide%d-cd%d", busid, devid); + ret = virAsprintf(&dev_name, "ide%d-cd%d", busid, devid); break; case VIR_DOMAIN_DISK_BUS_SCSI: if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) - ret = virAsprintf(&devname, "scsi%d-hd%d", busid, devid); + ret = virAsprintf(&dev_name, "scsi%d-hd%d", busid, devid); else - ret = virAsprintf(&devname, "scsi%d-cd%d", busid, devid); + ret = virAsprintf(&dev_name, "scsi%d-cd%d", busid, devid); break; case VIR_DOMAIN_DISK_BUS_FDC: - ret = virAsprintf(&devname, "floppy%d", devid); + ret = virAsprintf(&dev_name, "floppy%d", devid); break; case VIR_DOMAIN_DISK_BUS_VIRTIO: - ret = virAsprintf(&devname, "virtio%d", devid); + ret = virAsprintf(&dev_name, "virtio%d", devid); break; case VIR_DOMAIN_DISK_BUS_XEN: - ret = virAsprintf(&devname, "xenblk%d", devid); + ret = virAsprintf(&dev_name, "xenblk%d", devid); break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -508,7 +508,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) return -1; } - disk->info.alias = devname; + disk->info.alias = dev_name; return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d2626ff..c73b2ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10334,7 +10334,7 @@ cleanup: static int qemuDomainOpenConsole(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags) { @@ -10362,16 +10362,16 @@ qemuDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (devname) { + if (dev_name) { if (vm->def->console && - STREQ(devname, vm->def->console->info.alias)) + STREQ(dev_name, vm->def->console->info.alias)) chr = vm->def->console; for (i = 0 ; !chr && i < vm->def->nserials ; i++) { - if (STREQ(devname, vm->def->serials[i]->info.alias)) + if (STREQ(dev_name, vm->def->serials[i]->info.alias)) chr = vm->def->serials[i]; } for (i = 0 ; !chr && i < vm->def->nparallels ; i++) { - if (STREQ(devname, vm->def->parallels[i]->info.alias)) + if (STREQ(dev_name, vm->def->parallels[i]->info.alias)) chr = vm->def->parallels[i]; } } else { @@ -10384,14 +10384,14 @@ qemuDomainOpenConsole(virDomainPtr dom, if (!chr) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find character device %s"), - NULLSTR(devname)); + NULLSTR(dev_name)); goto cleanup; } if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("character device %s is not using a PTY"), - NULLSTR(devname)); + NULLSTR(dev_name)); goto cleanup; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fca8fa4..b4c43ae 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1217,7 +1217,7 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -1229,7 +1229,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *errs) { int ret; - VIR_DEBUG("mon=%p dev=%s", mon, devname); + VIR_DEBUG("mon=%p dev=%s", mon, dev_name); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1238,7 +1238,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONGetBlockStatsInfo(mon, devname, + ret = qemuMonitorJSONGetBlockStatsInfo(mon, dev_name, rd_req, rd_bytes, rd_total_times, wr_req, wr_bytes, @@ -1247,7 +1247,7 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, flush_total_times, errs); else - ret = qemuMonitorTextGetBlockStatsInfo(mon, devname, + ret = qemuMonitorTextGetBlockStatsInfo(mon, dev_name, rd_req, rd_bytes, rd_total_times, wr_req, wr_bytes, @@ -1282,17 +1282,17 @@ int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, } int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, unsigned long long *extent) { int ret; - VIR_DEBUG("mon=%p, fd=%d, devname=%p", - mon, mon->fd, devname); + VIR_DEBUG("mon=%p, fd=%d, dev_name=%p", + mon, mon->fd, dev_name); if (mon->json) - ret = qemuMonitorJSONGetBlockExtent(mon, devname, extent); + ret = qemuMonitorJSONGetBlockExtent(mon, dev_name, extent); else - ret = qemuMonitorTextGetBlockExtent(mon, devname, extent); + ret = qemuMonitorTextGetBlockExtent(mon, dev_name, extent); return ret; } @@ -1439,11 +1439,11 @@ int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online) int qemuMonitorEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force) { int ret; - VIR_DEBUG("mon=%p devname=%s force=%d", mon, devname, force); + VIR_DEBUG("mon=%p dev_name=%s force=%d", mon, dev_name, force); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1452,21 +1452,21 @@ int qemuMonitorEjectMedia(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONEjectMedia(mon, devname, force); + ret = qemuMonitorJSONEjectMedia(mon, dev_name, force); else - ret = qemuMonitorTextEjectMedia(mon, devname, force); + ret = qemuMonitorTextEjectMedia(mon, dev_name, force); return ret; } int qemuMonitorChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format) { int ret; - VIR_DEBUG("mon=%p devname=%s newmedia=%s format=%s", - mon, devname, newmedia, format); + VIR_DEBUG("mon=%p dev_name=%s newmedia=%s format=%s", + mon, dev_name, newmedia, format); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1475,9 +1475,9 @@ int qemuMonitorChangeMedia(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONChangeMedia(mon, devname, newmedia, format); + ret = qemuMonitorJSONChangeMedia(mon, dev_name, newmedia, format); else - ret = qemuMonitorTextChangeMedia(mon, devname, newmedia, format); + ret = qemuMonitorTextChangeMedia(mon, dev_name, newmedia, format); return ret; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 390eeff..477767f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -213,7 +213,7 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -227,7 +227,7 @@ int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, unsigned long long *extent); @@ -250,10 +250,10 @@ int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online); * this when doing the QMP implementation */ int qemuMonitorEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force); int qemuMonitorChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a81ec8..e38c2ed 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1338,7 +1338,7 @@ cleanup: int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -1405,12 +1405,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, /* New QEMU has separate names for host & guest side of the disk * and libvirt gives the host side a 'drive-' prefix. The passed - * in devname is the guest side though + * in dev_name is the guest side though */ if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STRNEQ(thisdev, devname)) + if (STRNEQ(thisdev, dev_name)) continue; found = 1; @@ -1485,7 +1485,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (!found) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), devname); + _("cannot find statistics for device '%s'"), dev_name); goto cleanup; } ret = 0; @@ -1570,7 +1570,7 @@ cleanup: } int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, unsigned long long *extent) { int ret = -1; @@ -1620,12 +1620,12 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, /* New QEMU has separate names for host & guest side of the disk * and libvirt gives the host side a 'drive-' prefix. The passed - * in devname is the guest side though + * in dev_name is the guest side though */ if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STRNEQ(thisdev, devname)) + if (STRNEQ(thisdev, dev_name)) continue; found = 1; @@ -1653,7 +1653,7 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, if (!found) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), devname); + _("cannot find statistics for device '%s'"), dev_name); goto cleanup; } ret = 0; @@ -1840,12 +1840,12 @@ cleanup: int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force) { int ret; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("eject", - "s:device", devname, + "s:device", dev_name, "b:force", force ? 1 : 0, NULL); virJSONValuePtr reply = NULL; @@ -1864,7 +1864,7 @@ int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format) { @@ -1872,13 +1872,13 @@ int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon, virJSONValuePtr cmd; if (format) cmd = qemuMonitorJSONMakeCommand("change", - "s:device", devname, + "s:device", dev_name, "s:target", newmedia, "s:arg", format, NULL); else cmd = qemuMonitorJSONMakeCommand("change", - "s:device", devname, + "s:device", dev_name, "s:target", newmedia, NULL); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index dfeba7e..673444f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -61,7 +61,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -74,7 +74,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, unsigned long long *extent); @@ -92,10 +92,10 @@ int qemuMonitorJSONSetBalloon(qemuMonitorPtr mon, int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, int cpu, int online); int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force); int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0b50ba2..cedf582 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -751,7 +751,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -766,7 +766,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, int ret = -1; char *dummy; const char *p, *eol; - int devnamelen = strlen(devname); + int devnamelen = strlen(dev_name); if (qemuMonitorHMPCommand (mon, "info blockstats", &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, @@ -808,12 +808,12 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, while (*p) { /* New QEMU has separate names for host & guest side of the disk * and libvirt gives the host side a 'drive-' prefix. The passed - * in devname is the guest side though + * in dev_name is the guest side though */ if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) p += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STREQLEN (p, devname, devnamelen) + if (STREQLEN (p, dev_name, devnamelen) && p[devnamelen] == ':' && p[devnamelen+1] == ' ') { eol = strchr (p, '\n'); @@ -880,7 +880,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, /* If we reach here then the device was not found. */ qemuReportError (VIR_ERR_INVALID_ARG, - _("no stats found for device %s"), devname); + _("no stats found for device %s"), dev_name); cleanup: VIR_FREE(info); @@ -959,7 +959,7 @@ int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, } int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - const char *devname ATTRIBUTE_UNUSED, + const char *dev_name ATTRIBUTE_UNUSED, unsigned long long *extent ATTRIBUTE_UNUSED) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1163,21 +1163,21 @@ int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, int online) int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force) { char *cmd = NULL; char *reply = NULL; int ret = -1; - if (virAsprintf(&cmd, "eject %s%s", force ? "-f " : "", devname) < 0) { + if (virAsprintf(&cmd, "eject %s%s", force ? "-f " : "", dev_name) < 0) { virReportOOMError(); goto cleanup; } if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s"), devname); + _("could not eject media on %s"), dev_name); goto cleanup; } @@ -1186,7 +1186,7 @@ int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, * No message is printed on success it seems */ if (c_strcasestr(reply, "device ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s: %s"), devname, reply); + _("could not eject media on %s: %s"), dev_name, reply); goto cleanup; } @@ -1200,7 +1200,7 @@ cleanup: int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format ATTRIBUTE_UNUSED) { @@ -1214,14 +1214,14 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, goto cleanup; } - if (virAsprintf(&cmd, "change %s \"%s\"", devname, safepath) < 0) { + if (virAsprintf(&cmd, "change %s \"%s\"", dev_name, safepath) < 0) { virReportOOMError(); goto cleanup; } if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not change media on %s"), devname); + _("could not change media on %s"), dev_name); goto cleanup; } @@ -1230,14 +1230,14 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, * No message is printed on success it seems */ if (c_strcasestr(reply, "device ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not change media on %s: %s"), devname, reply); + _("could not change media on %s: %s"), dev_name, reply); goto cleanup; } /* Could not open message indicates bad filename */ if (strstr(reply, "Could not open ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not change media on %s: %s"), devname, reply); + _("could not change media on %s: %s"), dev_name, reply); goto cleanup; } diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 7cc3172..4651a17 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -58,7 +58,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, long long *rd_req, long long *rd_bytes, long long *rd_total_times, @@ -71,7 +71,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, unsigned long long *extent); int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, @@ -88,10 +88,10 @@ int qemuMonitorTextSetBalloon(qemuMonitorPtr mon, int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, int online); int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, bool force); int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, - const char *devname, + const char *dev_name, const char *newmedia, const char *format); diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index c08a10b..80f9dd1 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -649,7 +649,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, { char *part_num = NULL; char *devpath = NULL; - char *devname, *srcname; + char *dev_name, *srcname; virCommandPtr cmd = NULL; bool isDevMapperDevice; int rc = -1; @@ -663,26 +663,26 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - devname = basename(devpath); + dev_name = basename(devpath); srcname = basename(pool->def->source.devices[0].path); - VIR_DEBUG("devname=%s, srcname=%s", devname, srcname); + VIR_DEBUG("dev_name=%s, srcname=%s", dev_name, srcname); isDevMapperDevice = virIsDevMapperDevice(devpath); - if (!isDevMapperDevice && !STRPREFIX(devname, srcname)) { + if (!isDevMapperDevice && !STRPREFIX(dev_name, srcname)) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Volume path '%s' did not start with parent " - "pool source device name."), devname); + "pool source device name."), dev_name); goto cleanup; } if (!isDevMapperDevice) { - part_num = devname + strlen(srcname); + part_num = dev_name + strlen(srcname); if (*part_num == 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse partition number from target " - "'%s'"), devname); + "'%s'"), dev_name); goto cleanup; } diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index f2e1419..4f3add3 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -125,7 +125,7 @@ cleanup: static int -virStorageBackendIsMultipath(const char *devname) +virStorageBackendIsMultipath(const char *dev_name) { int ret = 0; struct dm_task *dmt = NULL; @@ -140,7 +140,7 @@ virStorageBackendIsMultipath(const char *devname) goto out; } - if (dm_task_set_name(dmt, devname) == 0) { + if (dm_task_set_name(dmt, dev_name) == 0) { ret = -1; goto out; } @@ -172,7 +172,7 @@ out: static int -virStorageBackendGetMinorNumber(const char *devname, uint32_t *minor) +virStorageBackendGetMinorNumber(const char *dev_name, uint32_t *minor) { int ret = -1; struct dm_task *dmt; @@ -182,7 +182,7 @@ virStorageBackendGetMinorNumber(const char *devname, uint32_t *minor) goto out; } - if (!dm_task_set_name(dmt, devname)) { + if (!dm_task_set_name(dmt, dev_name)) { goto out; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 32f09d2..2b7219a 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2235,7 +2235,7 @@ cleanup: static int umlDomainOpenConsole(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags) { @@ -2262,7 +2262,7 @@ umlDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (devname) { + if (dev_name) { /* XXX support device aliases in future */ umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Named device aliases are not supported")); @@ -2276,13 +2276,13 @@ umlDomainOpenConsole(virDomainPtr dom, if (!chr) { umlReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find character device %s"), devname); + _("cannot find character device %s"), dev_name); goto cleanup; } if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { umlReportError(VIR_ERR_INTERNAL_ERROR, - _("character device %s is not using a PTY"), devname); + _("character device %s is not using a PTY"), dev_name); goto cleanup; } diff --git a/src/util/util.c b/src/util/util.c index 9556bdb..1ff287d 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2455,11 +2455,11 @@ virTimeMs(unsigned long long *ms) #if HAVE_LIBDEVMAPPER_H bool -virIsDevMapperDevice(const char *devname) +virIsDevMapperDevice(const char *dev_name) { struct stat buf; - if (!stat(devname, &buf) && + if (!stat(dev_name, &buf) && S_ISBLK(buf.st_mode) && dm_is_dm_major(major(buf.st_rdev))) return true; @@ -2467,7 +2467,7 @@ virIsDevMapperDevice(const char *devname) return false; } #else -bool virIsDevMapperDevice(const char *devname ATTRIBUTE_UNUSED) +bool virIsDevMapperDevice(const char *dev_name ATTRIBUTE_UNUSED) { return false; } diff --git a/src/util/util.h b/src/util/util.h index 908ba7b..c55e852 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -255,7 +255,7 @@ char *virTimestamp(void); int virTimeMs(unsigned long long *ms) ATTRIBUTE_NONNULL(1); -bool virIsDevMapperDevice(const char *devname) ATTRIBUTE_NONNULL(1); +bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); int virEmitXMLWarning(int fd, const char *name, diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 2c66143..9c96fca 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2119,7 +2119,7 @@ out: static int xenUnifiedDomainOpenConsole(virDomainPtr dom, - const char *devname, + const char *dev_name, virStreamPtr st, unsigned int flags) { @@ -2135,7 +2135,7 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (devname) { + if (dev_name) { /* XXX support device aliases in future */ xenUnifiedError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Named device aliases are not supported")); @@ -2159,7 +2159,7 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom, if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { xenUnifiedError(VIR_ERR_INTERNAL_ERROR, - _("character device %s is not using a PTY"), devname); + _("character device %s is not using a PTY"), dev_name); goto cleanup; } diff --git a/tools/console.c b/tools/console.c index 82ef537..0f85bc7 100644 --- a/tools/console.c +++ b/tools/console.c @@ -278,7 +278,7 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED, } -int vshRunConsole(virDomainPtr dom, const char *devname) +int vshRunConsole(virDomainPtr dom, const char *dev_name) { int ret = -1; struct termios ttyattr, rawattr; @@ -331,7 +331,7 @@ int vshRunConsole(virDomainPtr dom, const char *devname) if (!con->st) goto cleanup; - if (virDomainOpenConsole(dom, devname, con->st, 0) < 0) + if (virDomainOpenConsole(dom, dev_name, con->st, 0) < 0) goto cleanup; con->stdinWatch = virEventAddHandle(STDIN_FILENO, diff --git a/tools/console.h b/tools/console.h index 580268d..9b05ff1 100644 --- a/tools/console.h +++ b/tools/console.h @@ -25,7 +25,7 @@ # ifndef WIN32 -int vshRunConsole(virDomainPtr dom, const char *devname); +int vshRunConsole(virDomainPtr dom, const char *dev_name); # endif /* !WIN32 */ -- 1.7.3.4

2 1

[libvirt] [PATCH v2] doc: Add statment about permissions needed to do a core dump
by Peter Krempa 16 Sep '11

16 Sep '11

Documentation did not specify, that some permissions are required on target path for coredump for the user running the hypervisor. Diff to v1: - reword statements --- src/libvirt.c | 3 ++- tools/virsh.pod | 3 +++ 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c32c7a6..4fb200d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2777,7 +2777,8 @@ error: * * This method will dump the core of a domain on a given file for analysis. * Note that for remote Xen Daemon the file path will be interpreted in - * the remote host. + * the remote host. Hypervisors may require the user to manually ensure + * proper permissions on the file named by @to. * * If @flags includes VIR_DUMP_CRASH, then leave the guest shut off with * a crashed state after the dump completes. If @flags includes diff --git a/tools/virsh.pod b/tools/virsh.pod index 5114580..02726f3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -621,6 +621,9 @@ rather than merely left in a paused state. If I<--bypass-cache> is specified, the save will avoid the file system cache, although this may slow down the operation. +NOTE: Some hypervisors may require the user to manually ensure proper +permissions on file and path specified by argument I<corefilepath>. + =item B<dumpxml> I<domain-id> [I<--inactive>] [I<--security-info>] [I<--update-cpu>] -- 1.7.3.4

2 1

[libvirt] libvirt(-java): How to make vm.getDomain().getJobInfo() thread safe?
by Thomas Treutner 16 Sep '11

16 Sep '11

Hi, this is kind of a follow-up to an older question/discussion: https://www.redhat.com/archives/libvir-list/2010-July/msg00267.html As a result of that, I use a second thread for monitoring the live migration, taking actions (setting maxdowntime to a value that fits the situation) if necessary. Although I call getJobInfo() with a quite low frequency (once a second), problems are occuring frequently, like every 10th or 15th live migration. Problems range from exceptions that the domain is not running anymore to complete JVM crashes -> http://pastebin.com/jT6sXubu Recovery from exceptions doesn't seem to work perfectly, as they seem to trigger that connections to a host can't be shut down properly because there are still open references. Of course, in my monitoring thread I'm checking in every monitoring iteration if the domain object is not null, is still active, if the jobInfo is available yet etc. But, as I can not synchronize with vm.migrate(), there still a reasonable chance that migrate() just invalidates the current domain while I'm accessing it, no matter what I do. Do I miss something or is that correct? Any ideas how to reliably solve it? Is there some experience from virt-manager, where (in my quite old version) I assume at least the domain is read for cpu etc. stats while live migrating... regards, thomas

2 4

[libvirt] [PATCH 0/8] Add virDomainMigrateGetMaxSpeed API
by Jim Fehlig 16 Sep '11

16 Sep '11

As per previous request [1], this patch series introduces virDomainMigrateGetMaxSpeed API. Patches 1-5 contain usual new API stuff. In patch 6, qemuDomainMigrateSetMaxSpeed is made to work with inactive domains and set the new value in domain conf. Patch 7 changes migration speed to unlimited when target is a file *and* no user defined migration speed is set, and reverts to previous value after migration completes. Patch 8 considers user-defined migration speed in qemuMigrationRun() when explicit bandwidth is not provided in 'resource' paramter. Note that a patch to fix the remote protocol generator [2] is required for this patch series. [1] https://www.redhat.com/archives/libvir-list/2011-August/msg00224.html [2] https://www.redhat.com/archives/libvir-list/2011-August/msg01367.html Jim Fehlig (8): Add on_migrate element to domainXML Add public API for getting migration speed Add max migration bandwidth to domain_conf Impl virDomainMigrateGetMaxSpeed in qemu driver virsh: Expose virDomainMigrateGetMaxSpeed API Save migration speed to domain conf in qemuDomainMigrateSetMaxSpeed Set qemu migration speed unlimited when migrating to file Use max speed specified in domain conf when migrating docs/apibuild.py | 3 +- docs/formatdomain.html.in | 21 +++++++ docs/schemas/domain.rng | 13 +++++ include/libvirt/libvirt.h.in | 4 ++ python/generator.py | 1 + python/libvirt-override-api.xml | 6 ++ python/libvirt-override.c | 24 ++++++++ src/conf/domain_conf.c | 11 ++++ src/conf/domain_conf.h | 2 + src/driver.h | 6 ++ src/libvirt.c | 51 ++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 76 ++++++++++++++++++++------- src/qemu/qemu_migration.c | 28 +++++++++- src/qemu/qemu_migration.h | 3 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++- src/remote_protocol-structs | 9 +++ src/rpc/gendispatch.pl | 1 + tests/domainschemadata/migration-params.xml | 34 ++++++++++++ tools/virsh.c | 41 ++++++++++++++ tools/virsh.pod | 4 ++ 22 files changed, 333 insertions(+), 24 deletions(-) create mode 100644 tests/domainschemadata/migration-params.xml -- 1.7.5.4

4 26

[libvirt] domain shutdown requests
by Christian Benvenuti (benve) 15 Sep '11

15 Sep '11

When you shutdown a domain that is in PAUSED state, the shutdown will not take effect until you issue a resume cmd: # virsh suspend <domain_name> <domain_name> is now PAUSED # virsh shutdown <domain_name> <domain_name> does not shutdown, with the reason being that, for example in the case of a Linux Guest, the guest is not running and therefore there is no acpid running and able to process the shutdown request. Right? If now we issue a resume #virsh resume <domain_name> the guest initiates the (pending) shutdown (ie, acpid processes the pending request). Is this the expected behavior? Shouldn't virsh do something like this when it issues a shutdown request? if (domain.state == PAUSED) resume(domain) shutdown(domain) If yes, then I guess this logic should go - into the cmdShutdown routine if we want to make it available to virsh only, or - inside the driver's domainShutdown fn if we want to make it available to any libvirt user. One more minor comment. As of now, "virsh list" (or 'virsh domstate <domain>') does not provide any way of knowing if - a domain is being shutdown or - a domain has a pending shutdown request (like in the example above). Would it make any sense to enhance the output of list/domstate , for example to show something like this when libvirt issues a shutdown to a running domain Id Name State ---------------------------------- 3 FC15-2 running (shutdown in progress) Or something like this when it issues a shutdown to a PAUSED domain? Id Name State ---------------------------------- 3 FC15-2 PAUSED (shutdown request pending) Thanks /Chris

3 5

[libvirt] [test-API][PATCH] Fix typos for existing problems
by Wayne Sun 15 Sep '11

15 Sep '11

--- cases/basic_nodedevice.conf | 2 +- lib/nodedevAPI.py | 2 +- lib/storageAPI.py | 2 +- repos/nodedevice/detach.py | 22 +++++++++++----------- repos/storage/define_scsi_pool.py | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cases/basic_nodedevice.conf b/cases/basic_nodedevice.conf index cea431e..85a68f7 100644 --- a/cases/basic_nodedevice.conf +++ b/cases/basic_nodedevice.conf @@ -1,4 +1,4 @@ -nodedevice:dettach +nodedevice:detach pciaddress $testpci diff --git a/lib/nodedevAPI.py b/lib/nodedevAPI.py index 8d588c4..cc96909 100644 --- a/lib/nodedevAPI.py +++ b/lib/nodedevAPI.py @@ -185,7 +185,7 @@ class NodedevAPI: code = e.get_error_code() raise exception.LibvirtAPI(message, code) - def connect(self, name) + def connect(self, name): try: obj = self.lookup_by_name(name) return obj.connect() diff --git a/lib/storageAPI.py b/lib/storageAPI.py index 2d75a0b..6c9d286 100644 --- a/lib/storageAPI.py +++ b/lib/storageAPI.py @@ -231,7 +231,7 @@ class StorageAPI(object): code = e.get_error_code() raise exception.LibvirtAPI(message, code) - def delete_pool(self, poolname, flags = 1): + def delete_pool(self, poolname, flags = 0): try: pool_obj = self.get_pool_obj(poolname) return pool_obj.delete(flags) diff --git a/repos/nodedevice/detach.py b/repos/nodedevice/detach.py index ac9aa89..18e792e 100644 --- a/repos/nodedevice/detach.py +++ b/repos/nodedevice/detach.py @@ -1,13 +1,13 @@ #!/usr/bin/env python """this test case is used for testing - dettach a specific node device + detach a specific node device """ __author__ = 'Alex Jia: ajia(a)redhat.com' __date__ = 'Tue Apr 6, 2010' __version__ = '0.1.0' __credits__ = 'Copyright (C) 2009 Red Hat, Inc.' -__all__ = ['usage', 'check_node_dettach', 'dettach'] +__all__ = ['usage', 'check_node_detach', 'detach'] import os @@ -45,8 +45,8 @@ def usage(params): else: pass -def check_node_dettach(pciaddress): - """Check node device dettach result, if dettachment is successful, the +def check_node_detach(pciaddress): + """Check node device detach result, if detachment is successful, the device host driver should be hided and the device should be bound to pci-stub driver, argument 'address' is a address of the node device """ @@ -64,7 +64,7 @@ def check_node_dettach(pciaddress): driver = os.path.basename(retval) return driver -def dettach(dicts): +def detach(dicts): """Dettach a specific node device and bind it to pci-stub driver, argument 'dicts' is a dictionary type and includes 'pciaddress' key, whose value uniquely identify a pci address of the node device @@ -78,7 +78,7 @@ def dettach(dicts): pciaddress = dicts['pciaddress'] - original_driver = check_node_dettach(pciaddress) + original_driver = check_node_detach(pciaddress) logger.info("original device driver: %s" % original_driver) util = utils.Utils() @@ -120,22 +120,22 @@ def dettach(dicts): try: try: - logger.info("dettach the node device") + logger.info("detach the node device") nodeobj.dettach(device_name) - current_driver = check_node_dettach(pciaddress) + current_driver = check_node_detach(pciaddress) logger.info("current device driver: %s" % current_driver) if current_driver != original_driver and current_driver == pciback: - logger.info("the node %s device dettach is successful" \ + logger.info("the node %s device detach is successful" \ % device_name) test_result = True else: - logger.info("the node %s device dettach is failed" % device_name) + logger.info("the node %s device detach is failed" % device_name) test_result = False return 1 except LibvirtAPI, e: logger.error("API error message: %s, error code is %s" \ % (e.response()['message'], e.response()['code'])) - logger.error("Error: fail to dettach %s node device" % device_name) + logger.error("Error: fail to detach %s node device" % device_name) test_result = False return 1 finally: diff --git a/repos/storage/define_scsi_pool.py b/repos/storage/define_scsi_pool.py index a5193e2..6afbe01 100644 --- a/repos/storage/define_scsi_pool.py +++ b/repos/storage/define_scsi_pool.py @@ -140,7 +140,7 @@ def define_scsi_pool(params): logger.error("API error message: %s, error code is %s" \ % (e.response()['message'], e.response()['code'])) return 1 - finally + finally: conn.close() logger.info("closed hypervisor connection") -- 1.7.1

2 1

[libvirt] [test-API][PATCH v4] Add test module ownership_test
by Wayne Sun 15 Sep '11

15 Sep '11

* repos/domain/ownership_test.py Setting the dynamic_ownership value in /etc/libvirt/qemu.conf, check the ownership of file after domain save and restore. Test could be on local or root_squash nfs. The default owner of the saved domain file is qemu:qemu in this case. --- repos/domain/ownership_test.py | 316 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 316 insertions(+), 0 deletions(-) create mode 100644 repos/domain/ownership_test.py diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py new file mode 100644 index 0000000..74620f4 --- /dev/null +++ b/repos/domain/ownership_test.py @@ -0,0 +1,316 @@ +#!/usr/bin/env python +"""Setting the dynamic_ownership in /etc/libvirt/qemu.conf, + check the ownership of saved domain file. Test could be on + local or root_squash nfs. The default owner of the saved + domain file is qemu:qemu in this case. + domain:ownership_test + guestname + #GUESTNAME# + dynamic_ownership + enable|disable + use_nfs + enable|disable +""" + +__author__ = 'Wayne Sun: gsun(a)redhat.com' +__date__ = 'Mon Jul 25, 2011' +__version__ = '0.1.0' +__credits__ = 'Copyright (C) 2011 Red Hat, Inc.' +__all__ = ['ownership_test'] + +import os +import re +import sys + +QEMU_CONF = "/etc/libvirt/qemu.conf" +SAVE_FILE = "/mnt/test.save" +TEMP_FILE = "/tmp/test.save" + +def append_path(path): + """Append root path of package""" + if path not in sys.path: + sys.path.append(path) + +from lib import connectAPI +from lib import domainAPI +from utils.Python import utils +from exception import LibvirtAPI + +pwd = os.getcwd() +result = re.search('(.*)libvirt-test-API', pwd) +append_path(result.group(0)) + +def return_close(conn, logger, ret): + """close hypervisor connection and return the given value""" + conn.close() + logger.info("closed hypervisor connection") + return ret + +def check_params(params): + """Verify inputing parameter dictionary""" + logger = params['logger'] + keys = ['guestname', 'dynamic_ownership', 'use_nfs'] + for key in keys: + if key not in params: + logger.error("%s is required" %key) + return 1 + return 0 + +def check_domain_running(domobj, guestname, logger): + """ check if the domain exists, may or may not be active """ + guest_names = domobj.get_list() + + if guestname not in guest_names: + logger.error("%s doesn't exist or not running" % guestname) + return 1 + else: + return 0 + +def nfs_setup(util, logger): + """setup nfs on localhost + """ + logger.info("set nfs service") + cmd = "echo /tmp *$rw,root_squash$ >> /etc/exports" + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to config nfs export") + return 1 + + logger.info("restart nfs service") + cmd = "service nfs restart" + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to restart nfs service") + return 1 + else: + for i in range(len(out)): + logger.info(out[i]) + + return 0 + +def chown_file(util, filepath, logger): + """touch a file and setting the chown + """ + if os.path.exists(filepath): + os.remove(filepath) + + touch_cmd = "touch %s" % filepath + logger.info(touch_cmd) + ret, out = util.exec_cmd(touch_cmd, shell=True) + if ret: + logger.error("failed to touch a new file") + logger.error(out[0]) + return 1 + + logger.info("set chown of %s as 107:107" % filepath) + chown_cmd = "chown 107:107 %s" % filepath + ret, out = util.exec_cmd(chown_cmd, shell=True) + if ret: + logger.error("failed to set the ownership of %s" % filepath) + return 1 + + logger.info("set %s mode as 664" % filepath) + cmd = "chmod 664 %s" % filepath + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to set the mode of %s" % filepath) + return 1 + + return 0 + +def prepare_env(util, dynamic_ownership, use_nfs, logger): + """configure dynamic_ownership in /etc/libvirt/qemu.conf, + set chown of the file to save + """ + if dynamic_ownership == 'enable': + d_ownership = 1 + elif dynamic_ownership == 'disable': + d_ownership = 0 + else: + logger.error("wrong dynamic_ownership value") + return 1 + + logger.info("set the dynamic ownership in %s as %s" % \ + (QEMU_CONF, d_ownership)) + set_cmd = "echo dynamic_ownership = %s >> %s" % \ + (d_ownership, QEMU_CONF) + ret, out = util.exec_cmd(set_cmd, shell=True) + if ret: + logger.error("failed to set dynamic ownership") + return 1 + + logger.info("restart libvirtd") + restart_cmd = "service libvirtd restart" + ret, out = util.exec_cmd(restart_cmd, shell=True) + if ret: + logger.error("failed to restart libvirtd") + return 1 + else: + for i in range(len(out)): + logger.info(out[i]) + + if use_nfs == 'enable': + filepath = TEMP_FILE + elif use_nfs == 'disable': + filepath = SAVE_FILE + else: + logger.error("wrong use_nfs value") + return 1 + + ret = chown_file(util, filepath, logger) + if ret: + return 1 + + if use_nfs == 'enable': + ret = nfs_setup(util, logger) + if ret: + return 1 + + cmd = "setsebool virt_use_nfs 1" + logger.info(cmd) + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("Failed to setsebool virt_use_nfs") + return 1 + + logger.info("mount the nfs path to /mnt") + mount_cmd = "mount -o vers=3 127.0.0.1:/tmp /mnt" + ret, out = util.exec_cmd(mount_cmd, shell=True) + if ret: + logger.error("Failed to mount the nfs path") + for i in range(len(out)): + logger.info(out[i]) + return 1 + + return 0 + +def ownership_get(logger): + """check the ownership of file""" + + statinfo = os.stat(SAVE_FILE) + uid = statinfo.st_uid + gid = statinfo.st_gid + + logger.info("the uid and gid of %s is %s:%s" %(SAVE_FILE, uid, gid)) + + return 0, uid, gid + +def ownership_test(params): + """Save a domain to a file, check the ownership of + the file after save and restore + """ + # Initiate and check parameters + params_check_result = check_params(params) + if params_check_result: + return 1 + + logger = params['logger'] + guestname = params['guestname'] + dynamic_ownership = params['dynamic_ownership'] + use_nfs = params['use_nfs'] + + util = utils.Utils() + + # set env + logger.info("prepare the environment") + ret = prepare_env(util, dynamic_ownership, use_nfs, logger) + if ret: + logger.error("failed to prepare the environment") + return 1 + + # Connect to local hypervisor connection URI + uri = util.get_uri('127.0.0.1') + conn = connectAPI.ConnectAPI() + virconn = conn.open(uri) + + # save domain to the file + logger.info("save domain %s to the file %s" % (guestname, SAVE_FILE)) + domobj = domainAPI.DomainAPI(virconn) + + logger.info("check the domain state") + ret = check_domain_running(domobj, guestname, logger) + if ret: + return return_close(conn, logger, 1) + + try: + domobj.save(guestname, SAVE_FILE) + logger.info("Success save domain %s to %s" % (guestname, SAVE_FILE)) + except LibvirtAPI, e: + logger.error("API error message: %s, error code is %s" % \ + (e.response()['message'], e.response()['code'])) + logger.error("Error: fail to save domain %s to %s" % \ + (guestname, SAVE_FILE)) + return return_close(conn, logger, 1) + + logger.info("check the ownership of %s after save" % SAVE_FILE) + ret, uid, gid = ownership_get(logger) + if use_nfs == 'enable': + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + elif use_nfs == 'disable': + if dynamic_ownership == 'enable': + if uid == 0 and gid == 0: + logger.info("As expected, the chown changed to root:root") + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + elif dynamic_ownership == 'disable': + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + + # restore domain from file + logger.info("restore the domain from the file") + try: + domobj.restore(guestname, SAVE_FILE) + logger.info("Success restore domain %s from %s" % \ + (guestname, SAVE_FILE)) + except LibvirtAPI, e: + logger.error("API error message: %s, error code is %s" % \ + (e.response()['message'], e.response()['code'])) + logger.error("Error: fail to restore domain %s from %s" % \ + (guestname, SAVE_FILE)) + return return_close(conn, logger, 1) + + logger.info("check the ownership of %s after restore" % SAVE_FILE) + ret, uid, gid = ownership_get(logger) + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + else: + logger.error("The chown of %s is %s:%s, not change back as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + + return return_close(conn, logger, 0) + +def ownership_test_clean(params): + """clean testing environment""" + logger = params['logger'] + use_nfs = params['use_nfs'] + + util = utils.Utils() + + if use_nfs == 'enable': + if os.path.ismount("/mnt"): + umount_cmd = "umount /mnt" + ret, out = util.exec_cmd(umount_cmd, shell=True) + if ret: + logger.error("Failed to unmount the nfs path") + for i in range(len(out)): + logger.error(out[i]) + + filepath = TEMP_FILE + elif use_nfs == 'disable': + filepath = SAVE_FILE + + if os.path.exists(filepath): + os.remove(filepath) + -- 1.7.1

2 1

[libvirt] [PATCH] Fix typos for snapshot-create-as in virsh help cmd and doc
by Nan Zhang 15 Sep '11

15 Sep '11

* tools/virsh.c: Using VSH_OT_STRING instead of VSH_OT_ARGV for diskspec * tools/virsh.pod: Fix missing -- tag for snapshot-create-as in docs --- tools/virsh.c | 2 +- tools/virsh.pod | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3c6e65a..5c4610e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12630,7 +12630,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")}, {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")}, {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")}, - {"diskspec", VSH_OT_ARGV, 0, + {"diskspec", VSH_OT_STRING, 0, N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")}, {NULL, 0, 0, NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index e82567d..c82cc45 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1773,7 +1773,7 @@ by command such as B<destroy> or by internal guest action). =item B<snapshot-create-as> I<domain> {[I<--print-xml>] | [I<--no-metadata>] [I<--halt>]} [I<name>] [I<description>] -[I<--disk-only> [I<diskspec>]...] +[I<--disk-only>] [I<--diskspec>]... Create a snapshot for domain I<domain> with the given <name> and <description>; if either value is omitted, libvirt will choose a @@ -1784,8 +1784,8 @@ inactive state after the snapshot is created, and if I<--disk-only> is specified, the snapshot will not include vm state. The I<--disk-only> flag is used to request a disk-only snapshot. When -this flag is in use, the command can also take additional I<diskspec> -arguments to add <disk> elements to the xml. Each <diskspec> is in the +this flag is in use, the command can also take additional I<--diskspec> +arguments to add <disk> elements to the xml. Each I<--diskspec> is in the form B<disk[,snapshot=type][,driver=type][,file=name]>. To include a literal comma in B<disk> or in B<file=name>, escape it with a second comma. For example, a diskspec of "vda,snapshot=external,file=/path/to,,new" -- 1.7.4.4

3 8

[libvirt] [PATCH] doc: Add statment about permissions needed to do a core dump
by Peter Krempa 15 Sep '11

15 Sep '11

Documentation did not specify, that some permissions are required on target path for coredump for the user running the hypervisor. --- src/libvirt.c | 4 +++- tools/virsh.pod | 3 +++ 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c32c7a6..ee88d87 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2777,7 +2777,9 @@ error: * * This method will dump the core of a domain on a given file for analysis. * Note that for remote Xen Daemon the file path will be interpreted in - * the remote host. + * the remote host. Hypervisors may require execute and/or write permissions + * for destination path specified by argument @to for user running the + * hypervisor. * * If @flags includes VIR_DUMP_CRASH, then leave the guest shut off with * a crashed state after the dump completes. If @flags includes diff --git a/tools/virsh.pod b/tools/virsh.pod index e82567d..ba7af15 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -621,6 +621,9 @@ rather than merely left in a paused state. If I<--bypass-cache> is specified, the save will avoid the file system cache, although this may slow down the operation. +NOTE: Some hypervisors may require execute and/or write permissions on +I<corefilepath> for user running the hypervisor. + =item B<dumpxml> I<domain-id> [I<--inactive>] [I<--security-info>] [I<--update-cpu>] -- 1.7.3.4

2 2