- Devel - libvirt List Archives

[libvirt] [PATCH 0/3] Some minor IOThread API adjustments/checks
by John Ferlan 18 Aug '15

18 Aug '15

As a result of the review of a related issue for virDomainAddIOThread it was noted that it's only our implementation for qemu that doesn't want an iothread_id == 0, see: http://www.redhat.com/archives/libvir-list/2015-August/msg00310.html Continued IRC chats with the submittor of that patch discovered some inconsistencies which are dealt with in this patch series... Patch 1 - Remove the iothread_id arg check in virDomainPinIOThread. Since 0 was allowed by the API anyway and left up to the hypervisor. The argument is unsigned - so in reality all that was being checked was 0 - other similar callers work on 'int' values not 'unsigned int'. Patch 2 - Adjust the description in virDomainAddIOThread to remove the "positive non-zero value" and then rework the text to be clearer that it's up to the hypervisor to decide what values are legal and how to handle possible duplication. Patch 3 - Add check in qemu_driver Add/Del IOThread API for the invalid 0 value for iothread_id NOTE: It's not necessary to add a specific illegal argument check for iothread_id = 0 in qemuDomainPinIOThread since both the live and config paths will first search for the iothread_id value and fail if not found in the iothreadid list. I'm not opposed to adding a specific error message if so desired... John Ferlan (3): api: Remove check on iothread_id arg in virDomainPinIOThread api: Adjust comment for virDomainAddIOThread qemu: Add check for invalid iothread_id in qemuDomainChgIOThread src/libvirt-domain.c | 7 +++---- src/qemu/qemu_driver.c | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) -- 2.1.0

2 4

[libvirt] [PATCH 0/2] 2 small fix for domrename
by Luyao Huang 18 Aug '15

18 Aug '15

*** BLURB HERE *** Luyao Huang (2): virsh: fix always return false in domrename libvirt-domain: forbid use virDomainRename in readonly connection src/libvirt-domain.c | 1 + tools/virsh-domain.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) -- 1.8.3.1

3 4

[libvirt] [PATCH] qemuDomainRename: Report error if adding domain to domainObj list fails
by Michal Privoznik 18 Aug '15

18 Aug '15

So, domain renaming works like this: new domain name is added into the list of domain objects. Then, domain definition is updated. After that, old domain name is removed from the domain object list. Now, if the very firs step fails for some reason, no error is reported: virsh # domrename dummy dummy error: An error occurred, but the cause is unknown Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3683591..8e365bd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19950,8 +19950,11 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) + if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("could not add new name into internal list of domains")); goto endjob; + } if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) goto rollback; -- 2.4.6

2 7

[libvirt] [PATCH] build: fix mingw build
by Eric Blake 18 Aug '15

18 Aug '15

Ever since commit e44b0269, 64-bit mingw compilation fails with: ../../src/util/virprocess.c: In function 'virProcessGetPids': ../../src/util/virprocess.c:628:50: error: passing argument 4 of 'virStrToLong_i' from incompatible pointer type [-Werror=incompatible-pointer-types] if (virStrToLong_i(ent->d_name, NULL, 10, &tmp_pid) < 0) ^ In file included from ../../src/util/virprocess.c:59:0: ../../src/util/virstring.h:53:5: note: expected 'int *' but argument is of type 'pid_t * {aka long long int *}' int virStrToLong_i(char const *s, ^ cc1: all warnings being treated as errors Although mingw won't be using this function, it does compile the file, and the fix is relatively simple. * src/util/virprocess.c (virProcessGetPids): Don't assume pid_t fits in int. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under build-breaker rule. src/util/virprocess.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 8fa7a9b..77a038a 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -619,14 +619,16 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) goto cleanup; while ((value = virDirRead(dir, &ent, taskPath)) > 0) { + long long tmp; pid_t tmp_pid; /* Skip . and .. */ if (STRPREFIX(ent->d_name, ".")) continue; - if (virStrToLong_i(ent->d_name, NULL, 10, &tmp_pid) < 0) + if (virStrToLong_ll(ent->d_name, NULL, 10, &tmp) < 0) goto cleanup; + tmp_pid = tmp; if (VIR_APPEND_ELEMENT(*pids, *npids, tmp_pid) < 0) goto cleanup; -- 2.4.3

1 0

[libvirt] [PATCH v2 0/3] Some additional checks for virDomainRename
by Michal Privoznik 17 Aug '15

17 Aug '15

So after some discussion to my original approach, this is v2. Michal Privoznik (3): virHashAddOrUpdateEntry: Tunr @new_name into void * virHashAddEntry: Report error on duplicate key qemuDomainRename: Explicitly check if domain is renaming to itself src/qemu/qemu_driver.c | 6 ++++++ src/util/virhash.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.4.6

2 6

[libvirt] [PATCH] utils: Remove the logging of errors from virNetDevSendEthtoolIoctl
by Moshe Levi 17 Aug '15

17 Aug '15

This patch remove the logging of errors of ioctl api and instead let the caller to choose what errors to log --- src/util/virnetdev.c | 44 +++++++++++++------------------------------- 1 files changed, 13 insertions(+), 31 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 2f3690e..cf79e8d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3032,11 +3032,10 @@ static int virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) { int ret = -1; - int sock = -1; + int sock; virIfreq ifr; - sock = socket(AF_LOCAL, SOCK_DGRAM, 0); - if (sock < 0) { + if ((sock = socket(AF_LOCAL, SOCK_DGRAM, 0)) < 0) { virReportSystemError(errno, "%s", _("Cannot open control socket")); goto cleanup; } @@ -3045,26 +3044,9 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) strcpy(ifr.ifr_name, ifname); ifr.ifr_data = cmd; ret = ioctl(sock, SIOCETHTOOL, &ifr); - if (ret != 0) { - switch (errno) { - case EPERM: - VIR_DEBUG("ethtool ioctl: permission denied"); - break; - case EINVAL: - VIR_DEBUG("ethtool ioctl: invalid request"); - break; - case EOPNOTSUPP: - VIR_DEBUG("ethtool ioctl: request not supported"); - break; - default: - virReportSystemError(errno, "%s", _("ethtool ioctl error")); - goto cleanup; - } - } cleanup: - if (sock) - VIR_FORCE_CLOSE(sock); + VIR_FORCE_CLOSE(sock); return ret; } @@ -3081,12 +3063,12 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) static int virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) { - int ret = -1; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = cmd->data > 0 ? 1 : 0; - return ret; + if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { + virReportSystemError(errno, _("Cannot get device %s flags"), ifname); + return -1; + } + return cmd->data > 0 ? 1 : 0; } @@ -3103,12 +3085,12 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) static int virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) { - int ret = -1; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); - return ret; + if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { + virReportSystemError(errno, _("Cannot get device %s generic features"), ifname); + return -1; + } + return FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); } # endif -- 1.7.1

2 4

[libvirt] [PATCH] qemu: Resolve Coverity UNINIT
by John Ferlan 16 Aug '15

16 Aug '15

Coverity complained that 'vm' wasn't initialized before jumping to cleanup: and calling virDomainObjEndAPI if the VIR_STRDUP fails. Rather than initialize vm = NULL, I moved the VIR_STRDUP closer to usage and used endjob for goto. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3683591..a54a3dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19902,9 +19902,6 @@ static int qemuDomainRename(virDomainPtr dom, virCheckFlags(0, ret); - if (VIR_STRDUP(new_dom_name, new_name) < 0) - goto cleanup; - if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -19940,6 +19937,9 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } + if (VIR_STRDUP(new_dom_name, new_name) < 0) + goto endjob; + if (virAsprintf(&rename_log_msg, ": domain %s has been renamed to %s\n", vm->def->name, new_name) < 0) { goto endjob; -- 2.1.0

2 2

[libvirt] [PATCH] protocol: Don't use rename as a variable name
by Martin Kletzander 15 Aug '15

15 Aug '15

That causes shadowing errors on older compilers. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Pushed under the build-breaker rule. 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 770aa72dc0f6..92a92e2bfa24 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3237,7 +3237,7 @@ struct remote_domain_rename_args { }; struct remote_domain_rename_ret { - int rename; + int retcode; }; /*----- Protocol. -----*/ diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ca36dc96e634..ff99c0096941 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2690,7 +2690,7 @@ struct remote_domain_rename_args { u_int flags; }; struct remote_domain_rename_ret { - int rename; + int retcode; }; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, -- 2.5.0

1 0

[libvirt] [PATCH] Remove static from vshReadline when readline not exist
by Moshe Levi 15 Aug '15

15 Aug '15

This patch remove the static from the vshReadline which introduce in this commit 834c5720e4434f0bcc807bb1cf20855af63e24a3. In with readline function vshReadline is not static but without readline it defined static which cause compilation error. --- tools/vsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 03ff859..1a5b6e8 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2655,7 +2655,7 @@ vshReadlineDeinit(vshControl *ctl ATTRIBUTE_UNUSED) /* empty */ } -static char * +char * vshReadline(vshControl *ctl, const char *prompt) { char line[1024]; -- 1.7.1

2 1

[libvirt] [PATCH v3 0/3] virt-shell: v3 diff series
by Erik Skultety 14 Aug '15

14 Aug '15

v3: - renamed virshCommandOptTimeoutToMs - resolved conflicts caused by virsh block job handling refactor - generic commands implementation moved to vsh.c As usual, for testing purposes, everything is available on my remote branch https://github.com/eskultety/libvirt/tree/virt-shell Erik Skultety (3): virt-shell: Resolve conflicts and some forgotten substitution from v2 virt-shell: Support command history for individual clients virt-shell: Move generic commands implementation to vsh.c src/libvirt_private.syms | 1 + src/util/virstring.c | 32 ++++ src/util/virstring.h | 1 + tools/virsh-domain.c | 150 +++++++++--------- tools/virsh-network.c | 2 +- tools/virsh.c | 390 ++++++++++++----------------------------------- tools/virsh.h | 1 - tools/vsh.c | 242 ++++++++++++++++++++++++++--- tools/vsh.h | 13 +- 9 files changed, 441 insertions(+), 391 deletions(-) -- 2.4.3

2 8