September 2016 - Devel - libvirt List Archives

[libvirt] [PATCH] virt-admin: Fix some English grammar in the manpage
by Erik Skultety 08 Sep '16

08 Sep '16

Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- tools/virt-admin.pod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Pushed as trivial. diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 0195001..020eb51 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -159,7 +159,7 @@ currently connected to. =head1 SERVER COMMANDS -Following commands manipulate daemon's server internal configuration. +The following commands manipulate daemon's server internal configuration. The I<server> is specified by its name. =over 4 @@ -281,7 +281,7 @@ I<--max-clients>. =head1 CLIENT COMMANDS -Following commands provide management and monitoring of clients connected to +The following commands provide management and monitoring of clients connected to one of daemon's available servers. Clients are specified by their numeric ID which is obtained by listing all clients connected to a specified server (see command B<srv-clients-list>). -- 2.5.5

1 0

[libvirt] [libvirt-php][PATCH 0/8] Couple of php fixes
by Michal Privoznik 08 Sep '16

08 Sep '16

Pushed under php-autopush rule. Michal Privoznik (8): libvirt_domain_get_network_info: Don't overwrite retval libvirt_storagepool_get_autostart: Rework translate_counter_type: Rework libvirt_domain_block_commit: Reindent libvirt_domain_qemu_agent_command: Reindent libvirt_domain_disk_add: Reindent libvirt_domain_disk_remove: Reindent libvirt_list_storagepools: Reindent src/libvirt-php.c | 129 ++++++++++++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 68 deletions(-) -- 2.8.4

1 8

[libvirt] [libvirt-php][PATCH] Don't leak @mime in libvirt_domain_get_screenshot_api
by Michal Privoznik 08 Sep '16

08 Sep '16

When getting a screenshot of a domain via virDomainScreenshot() the MIME type of a image is returned as a string which needs to be freed by caller. However, we are not doing that anywhere. ==24748== 48 bytes in 2 blocks are definitely lost in loss record 123 of 171 ==24748== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==24748== by 0x90674AB: xdr_string (xdr.c:790) ==24748== by 0xCD3AF1D: xdr_remote_nonnull_string (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0x9067B34: xdr_reference (xdr_ref.c:84) ==24748== by 0x9067C50: xdr_pointer (xdr_ref.c:135) ==24748== by 0xCD3B284: xdr_remote_string (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD3D288: xdr_remote_domain_screenshot_ret (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD5168E: virNetMessageDecodePayload (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD47E91: virNetClientProgramCall (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD1EBD3: callFull.isra.3 (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCD29973: remoteDomainScreenshot (in /usr/lib64/libvirt.so.0.2003.0) ==24748== by 0xCCDC24C: virDomainScreenshot (in /usr/lib64/libvirt.so.0.2003.0) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under php-autopush rule. src/libvirt-php.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 8edcb10..57ef1a0 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -4767,8 +4767,10 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) } #ifndef EXTWIN - if (mkstemp(file) == 0) + if (mkstemp(file) == 0) { + free(mime); RETURN_FALSE; + } #endif if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) { @@ -4776,6 +4778,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) { virStreamFree(st); set_error_if_unset("Cannot get create file to save domain screenshot" TSRMLS_CC); + free(mime); RETURN_FALSE; } } @@ -4783,6 +4786,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) if (virStreamRecvAll(st, streamSink, &fd) < 0) { virStreamFree(st); set_error_if_unset("Cannot receive screenshot data" TSRMLS_CC); + free(mime); RETURN_FALSE; } @@ -4791,6 +4795,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) if (virStreamFinish(st) < 0) { virStreamFree(st); set_error_if_unset("Cannot close stream for domain" TSRMLS_CC); + free(mime); RETURN_FALSE; } @@ -4816,6 +4821,8 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) VIRT_ADD_ASSOC_STRING(return_value, "file", file); VIRT_ADD_ASSOC_STRING(return_value, "mime", mime); } + + free(mime); } /* -- 2.8.4

1 0

[libvirt] [PATCH] Avoid segfault in virt-aa-helper when handling read-only mount filesystems
by rufo 07 Sep '16

07 Sep '16

This patch fixes a segfault in virt-aa-helper caused by attempting to modify a string literal in situ. It is triggered when a domain has a <filesystem> with type='mount' configured readonly, and libvirt is using the AppArmor security driver for sVirt confinement. --- src/security/virt-aa-helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 49e12b9..c22aa66 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -740,6 +740,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi bool readonly = true; bool explicit_deny_rule = true; char *sub = NULL; + char *perms_new = strdup(perms); if (path == NULL) return rc; @@ -764,12 +765,12 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi return rc; } - if (strchr(perms, 'w') != NULL) { + if (strchr(perms_new, 'w') != NULL) { readonly = false; explicit_deny_rule = false; } - if ((sub = strchr(perms, 'R')) != NULL) { + if ((sub = strchr(perms_new, 'R')) != NULL) { /* Don't write the invalid R permission, replace it with 'r' */ sub[0] = 'r'; explicit_deny_rule = false; @@ -787,7 +788,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi if (tmp[strlen(tmp) - 1] == '/') tmp[strlen(tmp) - 1] = '\0'; - virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms); + virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms_new); if (explicit_deny_rule) { virBufferAddLit(buf, " # don't audit writes to readonly files\n"); virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : ""); -- 2.9.3

4 5

[libvirt] [PATCH] Delete extra wrap after vol-reisize error
by Yanqiu Zhang 07 Sep '16

07 Sep '16

This patch is to delete the extra wrap "\n" after failed vol-reisize error, both "Failed to change size of volume to" and "Failed to change size of volume by". For error with wrap, there will be an extra wrap between two errors, such as: (1)# virsh vol-resize --pool default --vol vol-test 5M error: Failed to change size of volume 'vol-test' to 5M error: invalid argument: Can't shrink capacity below current capacity unless shrink flag explicitly specified (2)# virsh vol-resize /var/lib/libvirt/images/volds --shrink --delta 10M error: Failed to change size of volume 'volds' by 10M error: invalid argument: can't shrink capacity below existing allocation --- tools/virsh-volume.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index d35fee1..4b3b4d7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1128,8 +1128,8 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) ret = true; } else { vshError(ctl, - delta ? _("Failed to change size of volume '%s' by %s\n") - : _("Failed to change size of volume '%s' to %s\n"), + delta ? _("Failed to change size of volume '%s' by %s") + : _("Failed to change size of volume '%s' to %s"), virStorageVolGetName(vol), capacityStr); ret = false; } -- 1.8.3.1

2 1

[libvirt] [libvirt-php][PATCH] libvirt_list_active_domains: Don't free domain before strduping its name
by Michal Privoznik 07 Sep '16

07 Sep '16

There's a bug in the implementation of libvirt_list_active_domains(). We try to return an array of names of active domains. So we iterate over array of domains as returned by libvirt and get each domain's name. But we don't obtain a duplicate of that name rather just a pointer into virDomain object. The very next thing we do is we free the virDomain object and then try to copy what's now invalid pointer. Reverse the order of those two actions. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libvirt-php.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 2045c59..8edcb10 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -9128,18 +9128,15 @@ PHP_FUNCTION(libvirt_list_active_domains) domain=virDomainLookupByID(conn->conn,ids[i]); if (domain!=NULL) { - name=virDomainGetName(domain); - - if (virDomainFree (domain)) - resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC); - - if (name==NULL) - { - efree (ids); + if (!(name = virDomainGetName(domain))) { + efree(ids); RETURN_FALSE; } VIRT_ADD_NEXT_INDEX_STRING(return_value, name); + + if (virDomainFree(domain)) + resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC); } } efree(ids); -- 2.8.4

2 1

[libvirt] [libvirt-php] libvirt_list_active_domains returns garbage querying a Hyper-V host
by Fernando Casas Schössow 07 Sep '16

07 Sep '16

Hi all, I'm using libvirt-php to manage a Hyper-V 2012 R2 host. When trying to retrieve the active VMs using the function libvirt_list_active_domains() I get an array with the correct amount of items but with garbage instead of the domain names: Array ( [0] => P’*µ [1] => +µ [2] => `„*µ [3] => °Ø*µ [4] => Ð„*µ [5] => `†*µ [6] => [7] => `†*µ [8] => [9] => p+µ [10] => p"+µ [11] => P#+µ [12] => ß*µ ) On the other hand libvirt_list_inactive_domains() and libvirt_list_domains() work as expected returning the right amount of items and the domain names correctly: libvirt_list_inactive_domains() output: Array ( [0] => DCHOMELAB01 [1] => VMFCSW7 [2] => REMOTEAPP01 [3] => MPSSD01 [4] => CS16SVR ) Sofware versions: OS: CentOS Linux release 7.2.1511 (Core) Kernel: 3.10.0-327.28.2.el7.x86_64 PHP: 5.4.16-36.3 libvirt: 2.1.0 libvirt-php: 0.5.2 Node: Hyper-V 2012 R2 Thanks in advance for your replies. Fer

3 5

[libvirt] [PATCH] qemu_driver: protect existed save image
by Chen Hanxiao 07 Sep '16

07 Sep '16

From: Chen Hanxiao <chenhanxiao(a)gmail.com> We may overwrite existed save image by 'virsh save' command. This patch will check qemu save headers before trying to rewrite it. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- src/qemu/qemu_driver.c | 141 +++++++++++++++++++++++++++++++------------------ 1 file changed, 89 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0ac2ef..56aa079 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2806,6 +2806,73 @@ bswap_header(virQEMUSaveHeaderPtr hdr) } +static int +qemuSaveHeaderCheck(int fd, + virQEMUSaveHeaderPtr header, + const char *path, + bool unlink_corrupt) +{ + int ret = -1; + + if (saferead(fd, header, sizeof(*header)) != sizeof(*header)) { + if (unlink_corrupt) { + if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) { + virReportSystemError(errno, + _("cannot remove corrupt file: %s"), + path); + goto error; + } + return -3; + } + virReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("failed to read qemu header")); + goto error; + } + + if (memcmp((*header).magic, QEMU_SAVE_MAGIC, sizeof((*header).magic)) != 0) { + const char *msg = _("image magic is incorrect"); + + if (memcmp((*header).magic, QEMU_SAVE_PARTIAL, + sizeof((*header).magic)) == 0) { + msg = _("save image is incomplete"); + if (unlink_corrupt) { + if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) { + virReportSystemError(errno, + _("cannot remove corrupt file: %s"), + path); + goto error; + } + return -3; + } + } + virReportError(VIR_ERR_OPERATION_FAILED, "%s", msg); + goto error; + } + + if ((*header).version > QEMU_SAVE_VERSION) { + /* convert endianess and try again */ + bswap_header(header); + } + + if ((*header).version > QEMU_SAVE_VERSION) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("image version is not supported (%d > %d)"), + (*header).version, QEMU_SAVE_VERSION); + goto error; + } + + if ((*header).xml_len <= 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("invalid XML length: %d"), (*header).xml_len); + goto error; + } + + ret = 0; + + error: + return ret; +} + /* return -errno on failure, or 0 on success */ static int qemuDomainSaveHeader(int fd, const char *path, const char *xml, @@ -2827,6 +2894,7 @@ qemuDomainSaveHeader(int fd, const char *path, const char *xml, _("failed to write xml to '%s'"), path); goto endjob; } + endjob: return ret; } @@ -3067,6 +3135,24 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, goto cleanup; } } + + /* don't overwrite exsited save files */ + if (virFileExists(path)) { + virQEMUSaveHeader header_tmp; + fd = qemuOpenFile(driver, NULL, path, O_RDONLY, NULL, NULL); + if (fd < 0) + goto cleanup; + + if (qemuSaveHeaderCheck(fd, &header_tmp, path, false) >= 0) { + ret = -EEXIST; + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to write existed image file '%s'"), + path); + goto cleanup; + } + VIR_FORCE_CLOSE(fd); + } + fd = qemuOpenFile(driver, vm, path, O_WRONLY | O_TRUNC | O_CREAT | directFlag, &needUnlink, &bypassSecurityDriver); @@ -6327,6 +6413,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, bool unlink_corrupt) { int fd = -1; + int ret = -1; virQEMUSaveHeader header; char *xml = NULL; virDomainDefPtr def = NULL; @@ -6353,58 +6440,8 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, VIR_FILE_WRAPPER_BYPASS_CACHE))) goto error; - if (saferead(fd, &header, sizeof(header)) != sizeof(header)) { - if (unlink_corrupt) { - if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) { - virReportSystemError(errno, - _("cannot remove corrupt file: %s"), - path); - goto error; - } - return -3; - } - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to read qemu header")); + if ((ret = qemuSaveHeaderCheck(fd, &header, path, unlink_corrupt)) < 0) goto error; - } - - if (memcmp(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)) != 0) { - const char *msg = _("image magic is incorrect"); - - if (memcmp(header.magic, QEMU_SAVE_PARTIAL, - sizeof(header.magic)) == 0) { - msg = _("save image is incomplete"); - if (unlink_corrupt) { - if (VIR_CLOSE(fd) < 0 || unlink(path) < 0) { - virReportSystemError(errno, - _("cannot remove corrupt file: %s"), - path); - goto error; - } - return -3; - } - } - virReportError(VIR_ERR_OPERATION_FAILED, "%s", msg); - goto error; - } - - if (header.version > QEMU_SAVE_VERSION) { - /* convert endianess and try again */ - bswap_header(&header); - } - - if (header.version > QEMU_SAVE_VERSION) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("image version is not supported (%d > %d)"), - header.version, QEMU_SAVE_VERSION); - goto error; - } - - if (header.xml_len <= 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("invalid XML length: %d"), header.xml_len); - goto error; - } if (VIR_ALLOC_N(xml, header.xml_len) < 0) goto error; @@ -6439,7 +6476,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, VIR_FORCE_CLOSE(fd); virObjectUnref(caps); - return -1; + return ret; } static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6) -- 1.8.3.1

2 2

[libvirt] NPIV storage pools do not map to same LUN units across hosts.
by Nitesh Konkar 07 Sep '16

07 Sep '16

Link: http://wiki.libvirt.org/page/NPIV_in_libvirt Topic: Virtual machine configuration change to use vHBA LUN There is a NPIV storage pool defined on two hosts and pool contains a total of 8 volumes, allocated from a storage device. Source: # virsh vol-list poolvhba0 Name Path ------------------------------------------------------------------------------ unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366 unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367 unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368 unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369 unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380 unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381 unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382 -------------------------------------------------------------------- Destination: -------------------------------------------------------------------- # virsh vol-list poolvhba0 Name Path ------------------------------------------------------------------------------ unit:0:0:0 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000380 unit:0:0:1 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000381 unit:0:0:2 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000382 unit:0:0:3 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000367 unit:0:0:4 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000368 unit:0:0:5 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000366 unit:0:0:6 /dev/disk/by-id/wwn-0x6005076802818bda300000000000036a unit:0:0:7 /dev/disk/by-id/wwn-0x6005076802818bda3000000000000369 -------------------------------------------------------------------- As you can see in the above output,the same set of eight LUNs from the storage server have been mapped, but the order that the LUNs are probed on each host is different, resulting in different unit names on the two different hosts . If the the guest XMLs is referencing its storage by "unit" number then is it safe to migrate such guests because the "unit number" is assigned by the driver according to the specific way it probes the storage and hence when you migrate these guests , it results in different unit names on the destination hosts. Thus the migrated guest gets mapped to the wrong LUNs and is given the wrong disks. The problem is that the LUN numbers on the destination host and source host do not agree. Example, LUN 0 on source_host, for example, may be LUN 5 on destination_host. When the guest is given the wrong disk, it suffers a fatal I/O error. (This is manifested as fatal I/O errors since the guest has no idea that its disks just changed out under it.)The migration does not take into account that the unit numbers do match on on the source and destination sides. So, should libvirt make sure that the guest domains reference NPIV pool volumes by their globally-unique wwn instead of by "unit" numbers? The guest XML references its storage by "unit" number. Eg:- <disk type='volume' device='lun'> <driver name='qemu' type='raw' cache='none'/> <source pool='poolvhba0' volume='unit:0:0:0'/> <backingStore/> <target dev='vdb' bus='virtio'/> <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> I am planning to write a patch for it. Any comments on the above observation/approach would be appreciated. Thanks, Nitesh.

2 1

[libvirt] [PATCH] virt-admin: Output srv-clients-set data as unsigned int rather than signed
by Erik Skultety 07 Sep '16

07 Sep '16

Unfortunately, commit a8962f70 only fixed first half of the reported issue of virt-admin outputting negative values where unsigned int is expected by BZ below, so this commit represents the other missing half of the fix. resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1356769 Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- Pushed under trivial rule. Erik tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 513054b..12ec057 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -857,7 +857,7 @@ cmdSrvClientsInfo(vshControl *ctl, const vshCmd *cmd) } for (i = 0; i < nparams; i++) - vshPrint(ctl, "%-20s: %d\n", params[i].field, params[i].value.ui); + vshPrint(ctl, "%-20s: %u\n", params[i].field, params[i].value.ui); ret = true; -- 2.5.5

1 0